On Tue, Jan 31, 2012 at 3:43 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Daniel Farina <daniel@heroku.com> writes:
>> On Tue, Jan 17, 2012 at 2:14 AM, Daniel Farina <daniel@heroku.com> wrote:
>>> See the attached patch. It has a detailed cover letter/comment at the
>>> top of the file.
>
>> I have amended that description to be more accurate.
>
> I looked at this a bit, and it seems to go considerably further than
> I had in mind: unless I've missed something, this will instantiate a
> couple of kilobytes of static data in every .c file that includes
> pg_crc.h, directly or indirectly. While that might be tolerable in an
> external project, there are going to be a a LOT of copies of that table
> in the backend, many of them unused. Did you check to see how much
> larger the backend executable got? For that matter, aren't there a lot
> of build warnings about unused static variables?
Ah, yes, I think my optimizations were off when building, or
something. I didn't get such verbosity at first, and then I remember
doing something slightly different and then getting a lot of output.
I didn't pay attention to the build size. I will investigate.
> It occurs to me that rather than an #ifdef symbol, it might be
> appropriate to put the constant table into a separate .h file,
> say pg_crc_tables.h, so that users would control it by including
> that file or not rather than an #ifdef symbol. This is pretty
> trivial but might look a bit nicer.
I agree, I was about to say "what about a preprocessor hack..." after
the last paragraph, but saw you beat me to the punch. I'll have a look soon.
--
fdr
