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

Reply via email to