On 02/08/2015 08:33 PM, Abhijit Menon-Sen wrote:
> At 2015-02-08 18:46:30 +0200, hlinnakangas@vmware.com wrote:
>>
>> So I propose to move pg_crc.c to src/common, and move the tables
>> from pg_crc_tables.h directly to pg_crc.c. Thoughts?
>
> Sounds fine to me.
Ok, I've committed a patch that just moves the existing code to
common/pg_crc.c. I also moved pg_crc.h from include/utils to
include/utils. That'll need any external programs to change their
#include accordingly, but I think it was worth that. include/common is
clearly the correct home for that file, and the only reason to keep it
in include/utils would've been for backwards-compatibility.
Attached is a rebased version of the slicing-by-8 patch. I've made some
cosmetic changes. Most notably, I turned the bswap32() function into a
macro. Better to avoid the overhead of a function call, and it also
avoids building the function altogether on little-endian systems that
don't need it.
I'll continue review this.
> At 2015-01-02 16:46:29 +0200, hlinnakangas@vmware.com wrote:
>>
>> Would it even make sense to keep the crc variable in different byte
>> order, and only do the byte-swap once in END_CRC32() ?
>
> ...this certainly does make a noticeable difference. Will investigate.
Do you have access to big-endian hardware to test this on? It seems like
an obvious optimization to shave off that one instruction from the hot
loop, but if it turns out not to make any measurable difference, I'd
prefer to keep the tables in the same order on big and little endian
systems, reducing the amount of #ifdefs needed.
I tested this on my laptop by adding a BSWAP32() into the hot loop -
which is bogus on a little endian Intel system - and it seems to make
about 5% difference in a quick micro-benchmark. But it would be nice to
get some numbers from the kind of big endian systems that people run in
the real world.
- Heikki
