phar: use crc32 bulk method instead. #6099
Conversation
frankdjx
force-pushed
the
phar_crc32
branch
2 times, most recently
from
e7225f8 to
6324ba0
Compare
|
@weltling Hi, could you help to review this also? Some phar code are using CRC32 macro which can do on;y one char computing based on lookup table, this commit change them to use a new bulk method which can benefit from the SIMD crc32. The SIMD computing is available on both x86 and arm platforms. |
frankdjx
force-pushed
the
phar_crc32
branch
from
6324ba0 to
d7279cb
Compare
|
What is the performance impact? |
I don't have one benchmark for it. But it should expect similar speed up as shown in #6018 |
There was a problem hiding this comment.
@jianxind thanks a lot for the patch, and looks like i completely overslept your ping. I'm also reviving an RP4 to do some test on ARM, should be able to finish this weekend.
There are some minor nits from my side, otherwise LGTM. Could you also rebase please, after you've checked?
Thanks!
Benefit from the hardware crc32 computing. Signed-off-by: Frank Du <frank.du@intel.com>
Signed-off-by: Frank Du <frank.du@intel.com>
| for (loc = 0;loc < mytime; ++loc) { | ||
| CRC32(newcrc32, php_stream_getc(file)); | ||
| } | ||
| crc32_stream_bulk_update(&newcrc32, file, entry->uncompressed_filesize); | ||
| entry->crc32 = ~newcrc32; |
There was a problem hiding this comment.
Seems we don't need check the return as it just generate a crc value here.
There was a problem hiding this comment.
This might be another plus to return the processed length and compare with uncompressed_filesize for a better security. Otherwise might make sense to (void) to silence possible static analyzer warnings.
thanks.
| for (loc = 0;loc < entry->uncompressed_filesize; ++loc) { | ||
| CRC32(newcrc32, php_stream_getc(efp)); | ||
| } | ||
| crc32_stream_bulk_update(&newcrc32, efp, entry->uncompressed_filesize); |
| *crc = crc32_bulk_update(*crc, buf, n); | ||
| handled += n; | ||
| } else { /* EOF */ | ||
| return FAILURE; |
There was a problem hiding this comment.
NIT EOF might be not directly an error, since it loops and EOL might be delivered not in the first iteration. Probably ok if it's only about local files, but stream can be anything (also remote). Perhaps it's better to return handled or EOF. Anyway this does the job and we don't export.
Thanks!
|
Thanks. |
Benefit from the hardware crc32 computing.
Signed-off-by: Frank Du frank.du@intel.com