Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:97610
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <87k2a5jcx5.fsf@lil.giraffy.jp>
References: <CAGa2bXZ9EzWHDTBg_ZHGW4KdjiFK4tpE7aMCVTZD5n+UZ2vEWA@mail.gmail.com>
 <7500b0c6a50baf49beac70ae01e8b50d@koti.fimnet.fi> <CAGa2bXaZPBYTaXzNDnA32BCBoZ_FzFUnRNYKFY8b1TXAWSwMyg@mail.gmail.com>
 <87o9zhju4d.fsf@lil.giraffy.jp> <CAGa2bXb0hxQ2A=5APHcDkjwwRESOpUf-kUL3AMaBYr7dTkrWZg@mail.gmail.com>
 <CAGa2bXZ3wFd3BXsSK-FVQVPQT5h1J-FPh=4AyU+4Zf52MmKm2g@mail.gmail.com> <87k2a5jcx5.fsf@lil.giraffy.jp>
Date: Mon, 9 Jan 2017 15:08:46 +0900
Message-ID: <CAGa2bXZgh6sS=9QTatRqZK7+33GmUZzZtTarzA_m7DBCWHwpNw@mail.gmail.com>
To: Kazuo Oishi <kazuo@o-ishi.jp>
Cc: =?UTF-8?Q?Lauri_Kentt=C3=A4?= <lauri.kentta@gmail.com>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a114d3d4abeca9d0545a33575
Subject: Re: [PHP-DEV] Use decent entropy for uniqid($prefix, TRUE)
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a114d3d4abeca9d0545a33575
Content-Type: text/plain; charset=UTF-8

Hi Kazuo,

On Mon, Jan 9, 2017 at 9:27 AM, Kazuo Oishi <kazuo@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
yohgaki@ohgaki.net

--001a114d3d4abeca9d0545a33575--