Hi Kazuo, On Mon, Jan 9, 2017 at 9:27 AM, Kazuo Oishi <ka...@o-ishi.jp> wrote:
> >>> [original uniqid() using php_combined_lcg] > >>> $ time ./php_uniqid_orig -r 'for($i=0; $i<1000000;$i++) > uniqid("",true);' > >>> real 0m0.366s > >>> user 0m0.350s > >>> sys 0m0.010s > >>> > >>> [your php_random_bytes_throw version (commit > >>> 48f1a17886d874dc90867c669481804de90509e8)] > >>> $ time ./php_uniqid_yohgaki -r 'for($i=0; $i<1000000;$i++) > >>> uniqid("",true);' > >>> real 0m4.509s > >>> user 0m0.430s > >>> sys 0m4.070s > >>> > >>> [Lauri's php_random_int version] > >>> $ time ./php_uniqid_lauri -r 'for($i=0; $i<1000000;$i++) > uniqid("",true);' > >>> real 0m0.664s > >>> user 0m0.260s > >>> sys 0m0.400s > >> > >> Interesting result. AFAIK, I didn't get significant difference when I > made > >> the patch. > >> What is your system? It seems your PRNG is significantly slow. > > Core i7-5600U 2.60GHz > Linux version 4.8.10, gcc version 4.9.3, gentoo > Thanks. I don't see such difference on my Fedora 25 Corei7-4770S gcc version 6.3.1 20161221 (Red Hat 6.3.1-1) (GCC) I'm curious because I don't see performance issue you have. I'll send patch next week or so because I'm interested in how modified patch will perform on your system. > > The performance will be improved by reducing multiple PRNG calls to 1. > > I'll modify patch later, could you test it with your system? > > Sure. But as you said, Lauri's version would be optimal. I wrote the same patch right after php_random_int() was implemented and didn't find any problem. I think I've posted benchmark result in the previous uniqid() discussion thread. So I checked my patch again and found it should be PHP-master]$ git diff diff --git a/ext/standard/uniqid.c b/ext/standard/uniqid.c index 22173ae..bbd0e0a 100644 --- a/ext/standard/uniqid.c +++ b/ext/standard/uniqid.c @@ -35,7 +35,7 @@ #include <sys/time.h> #endif -#include "php_lcg.h" +#include "php_random.h" #include "uniqid.h" /* {{{ proto string uniqid([string prefix [, bool more_entropy]]) @@ -78,7 +78,9 @@ PHP_FUNCTION(uniqid) * digits for usecs. */ if (more_entropy) { - uniqid = strpprintf(0, "%s%08x%05x%.8F", prefix, sec, usec, php_combined_lcg() * 10); + zend_long rand; + php_random_int(0, 999999999, &rand, 1); + uniqid = strpprintf(0, "%s%08x%05x%.8F", prefix, sec, usec, (double)rand/100000000); } else { uniqid = strpprintf(0, "%s%08x%05x", prefix, sec, usec); } Notice that int values are less than a billion which is inside of signed 32 bit int range. This version is as fast as php_combined_lcg() version on my system. Both versions executes a million uniqid() calls about 0.36 sec. $ php -r '$s = microtime(TRUE);for($i=0;$i<1000000;$i++) uniqid("", TRUE); echo microtime(TRUE) - $s;' 0.36102104187012 So above patch would be the final patch. I don't expect issues but if there is performace issue on some systems, we may consider Lauri's integer computation version. I should have been disturbed by something when I wrote the silly patch. Sorry for confusions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net