From 7c851042cba011df867e628f9dbd29ce65deb75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 6 Apr 2024 17:46:28 +0200 Subject: [PATCH] random: Add clarifying comments to the implementation of CombinedLCG The implementation is needlessly obfuscated. It's not immediately clear that MODMULT is a simple modular multiplication, despite its name. Specifically it's not clear which of the parameters is the second factor. Furthermore the stated period is off-by-one: A value of `0` is part of its own chain, thus it may not be included in the period of the underlying generators. --- ext/random/engine_combinedlcg.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ext/random/engine_combinedlcg.c b/ext/random/engine_combinedlcg.c index ebecf9975f9..32f9af7d072 100644 --- a/ext/random/engine_combinedlcg.c +++ b/ext/random/engine_combinedlcg.c @@ -27,8 +27,11 @@ /* * combinedLCG() returns a pseudo random number in the range of (0, 1). * The function combines two CGs with periods of - * 2^31 - 85 and 2^31 - 249. The period of this function - * is equal to the product of both primes. + * 2^31 - 85 - 1 and 2^31 - 249 - 1. The period of this function + * is equal to the product of the two underlying periods, divided + * by factors shared by the underlying periods, i.e. 2.3 * 10^18. + * + * see: https://library.sciencemadness.org/lanl1_a/lib-www/numerica/f7-1.pdf */ #define MODMULT(a, b, c, m, s) q = s / a; s = b * (s - a * q) - c * q; if (s < 0) s += m @@ -43,7 +46,9 @@ static php_random_result generate(void *state) php_random_status_state_combinedlcg *s = state; int32_t q, z; + /* s->state[0] = (s->state[0] * 40014) % 2147483563; */ MODMULT(53668, 40014, 12211, 2147483563L, s->state[0]); + /* s->state[1] = (s->state[1] * 40692) % 2147483399; */ MODMULT(52774, 40692, 3791, 2147483399L, s->state[1]); z = s->state[0] - s->state[1];