On a similarly superficial scan, I have no issues with this. The only tiny thing which gives me pause is https://github.com/php/php-src/pull/2453/files#diff-0bb62bbdac4073ae183a857299826533R24 as I don't recall 100% if that redefinition will leak out to other parts of the runtime (my instinct says it will, so that might need to live in a different spot).
As long as the unit tests continue to pass (I assume they do) and the licensing on the bundled library is favorable (it seems to be), it's just one fast implementation in exchange for a slow one. -Sara On Wed, Apr 5, 2017 at 10:45 AM, Christian Schneider <cschn...@cschneid.com> wrote: > Am 05.04.2017 um 04:31 schrieb Yasuo Ohgaki <yohg...@ohgaki.net>: >> PHP's sha3 seems slower than it could be. > > I created a pull request at https://github.com/php/php-src/pull/2453 for a > version using the KeccakCodePackage version from > https://github.com/gvanas/KeccakCodePackage which yields ~30 times faster > results for a simple test. > > I don't know whether the PHP maintainers prefer to keep the (very short) > inline reference implementation which is slow or whether the optimised > version from KeccakCodePackage with around a dozen files would be acceptable. > > I put the (part which is used from the) KeccakCodePackage code (without > modifications!) in a separate directory under ext/hash/sha3 and I also > implemented switching between a 64bit and a 32bit version. Don't know if this > is necessary and following all the guidelines. > > Oh, I also only superficially scanned > > https://github.com/gvanas/KeccakCodePackage#under-which-license-is-the-kcp-distributed > but it looked like it should be ok. Not an expert on this though ;-) > > Regards, > - Chris > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
