On 2017-01-09 08:08, Yasuo Ohgaki wrote:
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 can confirm that the integer version is faster (2,6 seconds vs 3,6
seconds for 1000000 uniqid calls). Testing on ARM (Raspberry Pi) gives
similar results. It's not surprising, since converting to float/double
is not free and formatting a floating-point number is also slower than
formatting an integer.
Using floating-point numbers has exactly zero benefits, while now at
least two people have reported that integers are faster. I think it's
also much easier to read and understand integers, and your bugs tell the
same tale. So do you have some actual arguments for your version, or is
this just ”not invented here”?
Also, I must say that I'm neither for nor agains this change in general;
I'm discussing only the implementation.
--
Lauri Kenttä
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
