Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:114796 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 56949 invoked from network); 9 Jun 2021 13:01:49 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 9 Jun 2021 13:01:49 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id C0BC1180531 for ; Wed, 9 Jun 2021 06:16:50 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 9 Jun 2021 06:16:50 -0700 (PDT) Received: by mail-ej1-f44.google.com with SMTP id he7so19025285ejc.13 for ; Wed, 09 Jun 2021 06:16:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=9PNDXjW0D9fbQQc5ixyrNyZT90LqoIXQXtNlSUJ39bM=; b=ACMEQlf9+t3FMH4dXbPk9p58z3Bim1qHkoUQrtvAWi00vfn+y0c5MYNRUcQDBoYTBh sTZFPpWHyRkFkIoV1SRJg/RwdWNk9BQE+/GUbAZ+XElJzrUH66+UQRAkO0JGDgthDoIu SVJC7ToHYGI2wCJhnoRPZ4lmDXCwEv3e+y70p3uvZnSgTxnOKoPLq0QdKpI91NKWTgsY QS8c75FrsyI5coayJQ3gJXfvFSVFXtrtA25O/hlw9WcS/oMGaH4jNkVb4BeSLOVEUP4b L4wEDDrLFRisrbiwZphL7r0SMt+eHrdAw3LP9ARR68YAq9Bb6wYTtyUmMEs2BleAMCQg IAkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=9PNDXjW0D9fbQQc5ixyrNyZT90LqoIXQXtNlSUJ39bM=; b=b9dI3XC1SyWCCxlQp5f5SFfcMkCq19e8mOxunOWIxYQtBT8Dik8QcrgjD231Msym8g PLjySqAZmMjD8anyEp8vvXcim8+UbUaEzRF2aHz0EAdFioQEz+03G4sJJw7sPcOZZ3kh Vav0Rwh2eOyewvZ2kpKrJtzkt76a6MV5dCU+nljfAmokXL359FiDQKZ/YLT1j6bWDpEo 10LtZWJIHWmJCJUjckE1eG6LQgctjmuG90dQS3dqv47gbyUSYweGrSLa6Z3fflrH77vy 6Q2GFVbvRbP4olmjJ58u0pdSvYFKGXSfyA0dt9YzkKzLbT6XkgEPvCRw286fvyTYZd1x N3tQ== X-Gm-Message-State: AOAM532DNnYZ43fHekJ1S0nvcEPwv2J+53DF3kxvjWxZB0PWR64qXTUj IKbtUDF66h+yEM/mDWoMk1EvhGeIk8rg4VTN9Uc= X-Google-Smtp-Source: ABdhPJwH5fWLCP/a67Nt8ULz+xhqIQW8DR3XRYuWRbkF4da8Z+CIjBWlFLKl18AsiD91PCcjFADy56Y/6l/D2AtRx1k= X-Received: by 2002:a17:907:206a:: with SMTP id qp10mr28307984ejb.309.1623244608625; Wed, 09 Jun 2021 06:16:48 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: Date: Wed, 9 Jun 2021 22:16:37 +0900 Message-ID: To: Guilliam Xavier , PHP internals Content-Type: multipart/alternative; boundary="00000000000047e20405c455164f" Subject: Re: [PHP-DEV] Re: [RFC] Under Discussion: Add Random class and RandomNumberGenerator interface From: zeriyoshi@gmail.com (Go Kudo) --00000000000047e20405c455164f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks Guilliam. > I'm not sure you really addressed https://externals.io/message/114561#114680 ? I thought I had solved this problem by implementing the `RandomNumberGenerator` interface. Accepting strings and objects as arguments is certainly complicated, but I thought it met the necessary requirements. From the aspect of static analysis, there seems to be no problem. Reverting to a full object-based approach would increase the cost of providing new RNGs from extensions. I think the string approach is superior in this respect. Nikita's `createDefault()` approach is certainly good, but I think it is still difficult for beginners to understand. I also thought about it for a while after that, and I think that the current implementation is the most appropriate for now. What do you think? > Couldn't the Random class simply implement `public function __clone(): void` (that would internally do like `$this->algo =3D clone $this->algo;`)? Implicitly cloning in areas where the user cannot interfere may cause problems when using objects that cannot be cloned. However, this may be overthinking it. The reason is that a `RandomNumberGenerator` that cannot be cloned should be implemented as algo in the first place. If there are no objections, I will change the implementation. > Indeed, a decision has to be made. If you throw an exception, we wouldn't have the "issue" of different results on 32- vs 64-bit machines, but XorShift128+ and Secure would be unusable on a 32-bit machine, right? I don't see any problem with the current implicit truncation behavior for this. Even if a user switches to a 64-bit environment, compatibility can be maintained by explicitly truncating the generated values. In addition, most of the other methods only use 32bit internally. If you are concerned about this, you should probably be concerned about the incompatibility with MT_RAND_PHP. That problem can also be compensated for with an extension that adds algo. Regards, Go Kudo 2021=E5=B9=B46=E6=9C=889=E6=97=A5(=E6=B0=B4) 19:07 Guilliam Xavier : > Hello Go Kudo, > > On Tue, Jun 8, 2021 at 2:33 PM Go Kudo wrote: > >> Hello iinternals. >> >> There doesn't seem to be much mention of it. >> But, that may be because it has been discussed well in advance. >> Thank you for participating in the discussion. >> > > I'm not sure you really addressed > https://externals.io/message/114561#114680 ? especially this part: > > """ > If you pick the second option, then you should consistently separate the > source for both extension-provided RNGs and user-provided ones. I don't > think it makes sense to have extension provided RNGs use a `new > Random(RANDOM_XORSHIFT128PLUS)` interface, while user-provided ones use > `new Random(new SomeRandomSource)`. Going down this route, it should be > `new Random(new XorShift128Plus)`. This is a pure design, which also mean= s > that it's more complicated, and may make simple usages harder (though the= re > is certainly room for a Random::createDefault() here.) > """ > > Now, that might sound like it would take us back to the "full OO" version > (with multiple classes), which was deemed too "user-*un*friendly" by > several people (including me :s), > *but* there are some new ideas to consider, e.g. (for the Random class): > > - add a `public static function createDefault(): self` as suggested by > Nikita, or > > - change `public function __construct` parameters to e.g. > `(string $algo =3D XorShift128Plus::class, ?int $seed =3D null, mixed > ...$extra_args)` > (that would internally do a `new $algo($seed, ...$extra_args)` to store= ), > to be called not as > `new Random(new UserDefinedRNG($seed_or_null, $foo, $bar))` but as > `new Random(UserDefinedRNG::class, $seed_or_null, $foo, $bar)`. > > Any new opinions (or other ideas)? > > >> Now, if there is no particular discussion on this, I will try to start t= he >> voting phase next week. >> Of course, I will contact you separately. >> >> However, I was looking back at the implementation and found only one poi= nt >> of concern. >> >> With the current implementation, the results of the following example wi= ll >> match. >> >> ```php >> $one =3D new Random(); >> $one->nextInt(); >> $two =3D clone $one; >> >> var_dump($one->nextInt() =3D=3D=3D $two->nextInt()); // true >> ``` >> >> But, this is not true for user implementations. >> >> ```php >> class UserRNG implements RandomNumberGenerator >> { >> protected int $current =3D 0; >> >> public function generate(): int >> { >> return ++$this->current; >> } >> } >> >> $rng =3D new UserRNG(); >> $one =3D new Random($rng); >> $one->nextInt(); >> $two =3D clone $one; >> >> var_dump($one->nextInt() =3D=3D=3D $two->nextInt()); // false >> ``` >> >> This is because `$rng` is kept as a normal property and is managed by th= e >> standard PHP mechanism. In other words, it is passed by reference. >> >> This is not consistent with the built-in RNG behavior. However, I don't >> see >> a problem with this behavior. >> I feel that the standard PHP behavior is preferable to changing the >> userland behavior in a specific way. >> >> I would like to solicit opinions. >> > > Couldn't the Random class simply implement `public function __clone(): > void` (that would internally do like `$this->algo =3D clone $this->algo;`= )? > > >> >> Regards, >> Go Kudo >> >> 2021=E5=B9=B46=E6=9C=881=E6=97=A5(=E7=81=AB) 23:28 Go Kudo : >> >> > Hello internals. >> > Thanks for continuing to participate in the discussion. >> > >> > I've sorted out the proposal, and finished writing and implementing th= e >> > RFC. >> > (Funny, I know.) I think it's time to start a discussion. >> > >> > https://wiki.php.net/rfc/rng_extension >> > https://github.com/php/php-src/pull/7079 >> > >> > The main changes since last time are as follows: >> > >> > - The ugly RANDOM_USER has been removed. >> > - RandomNumberGenerator interface has been added for user-defined RNGs= . >> > - Random class is now final. >> > - Random class now accepts a RandomNumberGenerator interface other tha= n >> > string as the first argument to the constructor. >> > - INI directive has been removed. In 32-bit environments, the result i= s >> > always truncated. >> > >> > What I'm struggling with now is the behavior when calling nextInt() in= a >> > 32-bit environment using a 64-bit RNG. It currently returns a truncate= d >> > result, which means that the code loses compatibility with the result = of >> > running on a 64-bit machine. >> > I was also considering throwing an exception, but which would be >> > preferable? >> > > Indeed, a decision has to be made. If you throw an exception, we wouldn't > have the "issue" of different results on 32- vs 64-bit machines, but > XorShift128+ and Secure would be unusable on a 32-bit machine, right? > How do other existing implementations handle this question? > > > >> > I would like to answer a few unanswered questions. >> > >> > > What is bias? >> > >> > I' ve confirmed that the bias issue in mt_rand has already been fixed >> and >> > is no longer a problem. >> > >> > This implementation copies most of it from mt_rand. Therefore, this >> > problem does not exist. >> > >> > Therefore, an equivalent method to mt_getrandmax() is no longer >> provided. >> > > Great! > > >> > >> > > uint64 & right-shift >> > >> > This is a specification of the RNG implementation. PHP does not have >> > unsigned integers, but RNG handles unsigned integers (to be precise, >> even >> > the sign bit is random). >> > >> > As mentioned above, PHP does not have unsigned integers, which means >> that >> > using the results generated by RNGs may cause compatibility problems i= n >> the >> > future. To avoid this, a 1-bit right shift is always required (similar >> to >> > mt_rand()). >> > > Good to know, thanks. > > Regards, > > -- > Guilliam Xavier > --00000000000047e20405c455164f--