Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118218
Content-Type: multipart/mixed; boundary="------------kkgjHJMXHl980jPoFcOyTGAL"
Message-ID: <9dc8105d-a26e-18e7-a858-a6360e78b1a6@bastelstu.be>
Date: Thu, 7 Jul 2022 18:43:56 +0200
MIME-Version: 1.0
Content-Language: en-US
To: Go Kudo <zeriyoshi@gmail.com>, Go Kudo <g-kudo@colopl.co.jp>,
 PHP internals <internals@lists.php.net>
References: <CAMs36Hf_PsPnCjwxTMvLP10cRU9WheLKoYnr7+i91j39RkmNNA@mail.gmail.com>
 <CABTDVhBTbYMRT0vXX9+kyZ5CEJifhbPO0DH47bU2fy6EZHcD7g@mail.gmail.com>
In-Reply-To: <CABTDVhBTbYMRT0vXX9+kyZ5CEJifhbPO0DH47bU2fy6EZHcD7g@mail.gmail.com>
Subject: Re: [PHP-DEV] [VOTE] Random Extension Improvement
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

--------------kkgjHJMXHl980jPoFcOyTGAL
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi

On 7/7/22 17:52, Go Kudo wrote:
> Implementation is now proceeding.
> It includes fixes to some of the issues that were pointed out previously.
> 
> https://github.com/php/php-src/pull/8094
> 
> `Randomizer::arrayPickKeys()` is currently not implemented for now, since
> it is most likely to be rejected.
> (Of course, we are ready to revert to the other items if they are also
> rejected.)
> 
> I want to see if it looks good to you.

I've reviewed the current implementation of the engines with a specific 
focus on the tests.

1. I have verified the Xoshiro256** tests against a pure PHP 
implementation (which I verified against the C reference implementation 
[1]).

2. I have verified the Pcg64OneseqXslRr64 pcgoneseq128xslrr64_value.phpt 
test against the reference C implementation [2] with the attached 
pcg-verification.c.

3. I have verified the Mt19937 mt_value.phpt test against the C++ 
standard library implementation [3] with the attached mt-verification.cc.

The three engines do what they are supposed to do. I'm not qualified to 
review with regard to PHP Internals best practices. It would be good if 
someone with the necessary expertise could proceed with that part of the 
review.

Best regards
Tim Düsterhus

[1] https://prng.di.unimi.it/xoshiro256starstar.c
[2] https://www.pcg-random.org/download.html#c-implementation
[3] https://en.cppreference.com/w/cpp/numeric/random/mersenne_twister_engine
--------------kkgjHJMXHl980jPoFcOyTGAL
Content-Type: text/x-csrc; charset=UTF-8; name="pcg-verification.c"
Content-Disposition: attachment; filename="pcg-verification.c"
Content-Transfer-Encoding: base64

I2luY2x1ZGUgPHN0ZGRlZi5oPgojaW5jbHVkZSA8c3RkaW8uaD4KI2luY2x1ZGUgImluY2x1
ZGUvcGNnX3ZhcmlhbnRzLmgiCgppbnQKbWFpbih2b2lkKSB7CglzdHJ1Y3QgcGNnX3N0YXRl
XzEyOCBzOwoKCXBjZ19vbmVzZXFfMTI4X3NyYW5kb21fcigmcywgMTIzNCk7Cglmb3IgKHNp
emVfdCBpID0gMDsgaSA8IDEwMDAwOyBpKyspCgkJcGNnX29uZXNlcV8xMjhfeHNsX3JyXzY0
X3JhbmRvbV9yKCZzKTsKCQoJZm9yIChzaXplX3QgaSA9IDA7IGkgPCAxMjM0NTY3OyBpKysp
CgkJcGNnX29uZXNlcV8xMjhfeHNsX3JyXzY0X3JhbmRvbV9yKCZzKTsKCQoJdWludDY0X3Qg
cmVzdWx0ID0gcGNnX29uZXNlcV8xMjhfeHNsX3JyXzY0X3JhbmRvbV9yKCZzKTsKCglwcmlu
dGYoIkV4cGVjdGVkOiBiODhlMmEwZjIzNzIwYTFkXG4iKTsKCXByaW50ZigiQWN0dWFsICA6
ICIpOwoJZm9yIChzaXplX3QgaSA9IDA7IGkgPCA2NDsgaSArPSA4KQoJCXByaW50ZigiJTAy
bGx4IiwgKHJlc3VsdCA+PiBpKSAmIDB4ZmZVTEwpOwoJcHJpbnRmKCJcbiIpOwp9Cg==
--------------kkgjHJMXHl980jPoFcOyTGAL
Content-Type: text/x-c++src; charset=UTF-8; name="mt-verification.cc"
Content-Disposition: attachment; filename="mt-verification.cc"
Content-Transfer-Encoding: base64

I2luY2x1ZGUgPHJhbmRvbT4KI2luY2x1ZGUgPGlvc3RyZWFtPgoKaW50Cm1haW4odm9pZCkg
ewoJc3RkOjptdDE5OTM3IGc7CglnLnNlZWQoMTIzNCk7CgoJZm9yIChhdXRvIGkgPSAwOyBp
IDwgMTAwMDA7IGkrKykgZygpOwoKCWludDMyX3QgcmVzdWx0ID0gZygpOwoKCXN0ZDo6Y291
dCA8PCAiRXhwZWN0ZWQ6IDYwZmU5NWQ5IiA8PCBzdGQ6OmVuZGw7CglzdGQ6OmNvdXQgPDwg
IkFjdHVhbCAgOiAiOwoJZm9yIChhdXRvIGkgPSAwOyBpIDwgMzI7IGkgKz0gOCkKCQlzdGQ6
OmNvdXQgPDwgc3RkOjpoZXggPDwgKChyZXN1bHQgPj4gaSkgJiAweGZmKTsKCXN0ZDo6Y291
dCA8PCBzdGQ6OmVuZGw7Cn0K

--------------kkgjHJMXHl980jPoFcOyTGAL--