Hi Christian,

On Thu, Apr 6, 2017 at 12:45 AM, Christian Schneider <cschn...@cschneid.com>
wrote:

>
> 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.
>

Nice work!!
SHA3 is now faster than SHA2.

==== Small String ====
[yohgaki@dev php-src]$ time ./php-bin -r '$s = microtime(true); for ($i =
0;  $i < 1000000; $i++) { hash("sha256", "abcdefg"); }
var_dump(microtime(true) - $s); '
float(0.46163821220398)

real    0m0.557s
user    0m0.471s
sys    0m0.086s
[yohgaki@dev php-src]$ time ./php-bin -r '$s = microtime(true); for ($i =
0;  $i < 1000000; $i++) { hash("sha3-256", "abcdefg"); }
var_dump(microtime(true) - $s); '
float(0.57230806350708)

real    0m0.585s
user    0m0.579s
sys    0m0.006s

==== Large String ====
[yohgaki@dev php-src]$ time ./php-bin -r '$s = microtime(true);$v =
str_repeat("a", 9999999); for ($i = 0;  $i < 100; $i++) { hash("sha256",
$v); } var_dump(microtime(true) - $s); '
float(4.6689560413361)

real    0m4.691s
user    0m4.675s
sys    0m0.009s
[yohgaki@dev php-src]$ time ./php-bin -r '$s = microtime(true);$v =
str_repeat("a", 9999999); for ($i = 0;  $i < 100; $i++) { hash("sha3-256",
$v); } var_dump(microtime(true) - $s); '
float(2.9090809822083)

real    0m2.928s
user    0m2.919s
sys    0m0.005s



> 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 ;-)
>

It's fine for me.

I'll wait few weeks for more comments.
If not, I'll merge your PR to master.
I don't think we really need to merge it to released version.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

Reply via email to