ext/hash: optional support for the crc-fast library#20513
ext/hash: optional support for the crc-fast library#20513m6w6 wants to merge 3 commits intophp:masterfrom
Conversation
Dramatically increases the performance of all CRC-32 calculations on x86_64 and aarch64 platforms. The latest version of crc-fast (1.6.0 as of this writing) can exceed 110GiB/s on modern hardware. Adds CRC-64 support (both NVME and ECMA-182 variants, the two most popular, used in places like the Linux kernel, AWS S3, etc.), including software fallbacks if the crc-fast library isn’t used. Adds improved industry-standard naming for PHP’s supported CRC-32 variants, since PHP supports both standard (CRC-32/ISCSI and CRC-32/ISO-HDLC) and non-standard calculations but uses confusing, non-standard, and inconsistent naming (`crc32()` and `hash(‘crc32’)` are not the same thing!). The older, non-standard, confusing names can be deprecated at some point in the future, if desired.
|
that is quite a tall change :) that needs to be discussed first however (internal mailing list) at least, that sounds RFC material IMHO too. |
|
Looking into the test failure. Looks like unintneional serialization changes. |
|
Yeah I think this requires RFC possibly including some benchmarks. There's a precedent in #13194 that also requires RFC. The test will need fixing. |
|
I know that blake is a bit more contentious because it bundles the implementation and this is just lib. I personally don't have objections to add this but it's quite a lot of code so think it should go through RFC. |
- Uses int32_t instead of zend_long for consistency across 32-bit and 64-bit systems. - Updates test coverage to use the original crc32* values - Updates test coverage to use correct crc64* values
Multiline macros are painful for debugging, as pointed out on the PR. See: php#20513 (comment)
|
|
||
| /* CRC-64/NVME lookup table (reflected polynomial 0x9A6C9329AC4BC9B5) */ | ||
| static const uint64_t crc64_nvme_table[] = { | ||
| 0x0000000000000000ULL,0x7F6EF0C830358979ULL, |
There was a problem hiding this comment.
Are this tables guaranteed to be fully tested? If not, can they or are they from some upstream repo and can they be tested using #19802 ?
5 participants
Dramatically increases the performance of all CRC-32 calculations on x86_64 and aarch64 platforms. The latest version of crc-fast (1.6.0 as of this writing) can exceed 110GiB/s on modern hardware.
Adds CRC-64 support (both NVME and ECMA-182 variants, the two most popular, used in places like the Linux kernel, AWS S3, etc.), including software fallbacks if the crc-fast library isn’t used.
Adds improved industry-standard naming for PHP’s supported CRC-32 variants, since PHP supports both standard (CRC-32/ISCSI and CRC-32/ISO-HDLC) and non-standard calculations but uses confusing, non-standard, and inconsistent naming (
crc32()andhash(‘crc32’)are not the same thing!). The older, non-standard, confusing names can be deprecated at some point in the future, if desired.