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?
I think we'd be better off to continue to instantiate the table in just
one file in the backend. I'm not sure whether there'd be multiple
copies in libpq, but that would be appropriate to worry about too.
I am not sure if we'd still need the CRCDLLIMPORT hack or not in that
scenario.
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.
regards, tom lane
