From 167ac704c8387c531630949860e33fb5a59789a8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 Jul 2023 16:20:59 -0400 Subject: SL-18837: Clean up some redundancy in llrand.cpp. --- indra/llcommon/llrand.cpp | 96 +++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 41 deletions(-) (limited to 'indra/llcommon/llrand.cpp') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index cb28a8f5c3..4e4345f37a 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -58,14 +58,18 @@ * to restore uniform distribution. */ +template +inline REAL ll_internal_random(); + // *NOTE: The system rand implementation is probably not correct. #define LL_USE_SYSTEM_RAND 0 +/***************************************************************************** +* The LL_USE_SYSTEM_RAND implementation has been disabled since June 2008. +*****************************************************************************/ #if LL_USE_SYSTEM_RAND #include -#endif -#if LL_USE_SYSTEM_RAND class LLSeedRand { public: @@ -79,25 +83,25 @@ public: } }; static LLSeedRand sRandomSeeder; -inline F64 ll_internal_random_double() + +template <> +inline F64 ll_internal_random() { #if LL_WINDOWS - return (F64)rand() / (F64)RAND_MAX; + return (F64)rand() / (F64)(RAND_MAX+1); #else return drand48(); #endif } -inline F32 ll_internal_random_float() -{ -#if LL_WINDOWS - return (F32)rand() / (F32)RAND_MAX; -#else - return (F32)drand48(); -#endif -} -#else + +/***************************************************************************** +* This is the implementation we've been using. +*****************************************************************************/ +#else // LL_USE_SYSTEM_RAND static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); -inline F64 ll_internal_random_double() + +template <> +inline F64 ll_internal_random() { // *HACK: Through experimentation, we have found that dual core // CPUs (or at least multi-threaded processes) seem to @@ -107,16 +111,40 @@ inline F64 ll_internal_random_double() if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } +#endif // LL_USE_SYSTEM_RAND + +/***************************************************************************** +* Functions common to both implementations +*****************************************************************************/ +template <> +inline F32 ll_internal_random() +{ + return F32(ll_internal_random()); +} + +/*------------------------------ F64 aliases -------------------------------*/ +inline F64 ll_internal_random_double() +{ + return ll_internal_random(); +} + +F64 ll_drand() +{ + return ll_internal_random_double(); +} +/*------------------------------ F32 aliases -------------------------------*/ inline F32 ll_internal_random_float() { - // The clamping rules are described above. - F32 rv = (F32)gRandomGenerator(); - if(!((rv >= 0.0f) && (rv < 1.0f))) return fmod(rv, 1.f); - return rv; + return ll_internal_random(); } -#endif +F32 ll_frand() +{ + return ll_internal_random_float(); +} + +/*-------------------------- clamped random range --------------------------*/ S32 ll_rand() { return ll_rand(RAND_MAX); @@ -130,42 +158,28 @@ S32 ll_rand(S32 val) return rv; } -F32 ll_frand() -{ - return ll_internal_random_float(); -} - -F32 ll_frand(F32 val) +template +REAL ll_grand(REAL val) { // The clamping rules are described above. - F32 rv = ll_internal_random_float() * val; + REAL rv = ll_internal_random() * val; if(val > 0) { - if(rv >= val) return 0.0f; + if(rv >= val) return REAL(); } else { - if(rv <= val) return 0.0f; + if(rv <= val) return REAL(); } return rv; } -F64 ll_drand() +F32 ll_frand(F32 val) { - return ll_internal_random_double(); + return ll_grand(val); } F64 ll_drand(F64 val) { - // The clamping rules are described above. - F64 rv = ll_internal_random_double() * val; - if(val > 0) - { - if(rv >= val) return 0.0; - } - else - { - if(rv <= val) return 0.0; - } - return rv; + return ll_grand(val); } -- cgit v1.2.3 From 14d0b514af4e70956ecc2fbf6fe4c2e745e144a8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 18 Jul 2023 09:45:00 -0400 Subject: SL-18837: Ditch inactive llrand.cpp LL_USE_SYSTEM_RAND code. LL_USE_SYSTEM_RAND has been disabled since June 2008; that code only clutters the implementation we actually use. --- indra/llcommon/llrand.cpp | 46 +++------------------------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) (limited to 'indra/llcommon/llrand.cpp') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index 4e4345f37a..33afc50cf7 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -58,48 +58,12 @@ * to restore uniform distribution. */ +static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); + +// no default implementation, only specific F64 and F32 specializations template inline REAL ll_internal_random(); -// *NOTE: The system rand implementation is probably not correct. -#define LL_USE_SYSTEM_RAND 0 - -/***************************************************************************** -* The LL_USE_SYSTEM_RAND implementation has been disabled since June 2008. -*****************************************************************************/ -#if LL_USE_SYSTEM_RAND -#include - -class LLSeedRand -{ -public: - LLSeedRand() - { -#if LL_WINDOWS - srand(LLUUID::getRandomSeed()); -#else - srand48(LLUUID::getRandomSeed()); -#endif - } -}; -static LLSeedRand sRandomSeeder; - -template <> -inline F64 ll_internal_random() -{ -#if LL_WINDOWS - return (F64)rand() / (F64)(RAND_MAX+1); -#else - return drand48(); -#endif -} - -/***************************************************************************** -* This is the implementation we've been using. -*****************************************************************************/ -#else // LL_USE_SYSTEM_RAND -static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); - template <> inline F64 ll_internal_random() { @@ -111,11 +75,7 @@ inline F64 ll_internal_random() if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } -#endif // LL_USE_SYSTEM_RAND -/***************************************************************************** -* Functions common to both implementations -*****************************************************************************/ template <> inline F32 ll_internal_random() { -- cgit v1.2.3 From d255c3dda852731b6709ac4e9c9821b3be84ec86 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 14 Nov 2023 20:29:51 -0500 Subject: DRTVWR-588: Try to fix sporadic llrand test failures. With GitHub viewer builds, every few weeks we've seen test failures when ll_frand() returns exactly 1.0. This is a problem for a function that's supposed to return [0.0 .. 1.0). Monty suggests that the problem is likely to be conversion of F32 to F64 to pass to fmod(), and then truncation of fmod()'s F64 result back to F32. Moved the clamping code to each size-specific ll_internal_random specialization. Monty also noted that a stateful static random number engine isn't thread-safe. Added a mutex lock. --- indra/llcommon/llrand.cpp | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llrand.cpp') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index 33afc50cf7..20e25177f0 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -28,6 +28,7 @@ #include "llrand.h" #include "lluuid.h" +#include "mutex.h" /** * Through analysis, we have decided that we want to take values which @@ -58,8 +59,17 @@ * to restore uniform distribution. */ +static std::mutex gRandomGeneratorMutex; static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); +inline F64 ll_internal_random_unclamped() +{ + // gRandomGenerator is a stateful static object, which is therefore not + // inherently thread-safe. Lock it before use. + std::unique_lock lk(gRandomGeneratorMutex); + return gRandomGenerator(); +} + // no default implementation, only specific F64 and F32 specializations template inline REAL ll_internal_random(); @@ -71,7 +81,7 @@ inline F64 ll_internal_random() // CPUs (or at least multi-threaded processes) seem to // occasionally give an obviously incorrect random number -- like // 5^15 or something. Sooooo, clamp it as described above. - F64 rv = gRandomGenerator(); + F64 rv{ ll_internal_random_unclamped() }; if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } @@ -79,7 +89,13 @@ inline F64 ll_internal_random() template <> inline F32 ll_internal_random() { - return F32(ll_internal_random()); + // *HACK: clamp the result as described above. + // Per Monty, it's important to clamp using the correct fmodf() rather + // than expanding to F64 for fmod() and then truncating back to F32. Prior + // to this change, we were getting sporadic ll_frand() == 1.0 results. + F32 rv{ ll_internal_random_unclamped() }; + if(!((rv >= 0.0) && (rv < 1.0))) return fmodf(rv, 1.0f); + return rv; } /*------------------------------ F64 aliases -------------------------------*/ -- cgit v1.2.3 From d427d5dbfa09f0bdec743e75a41e8ea0ee4abeea Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 15 Nov 2023 10:12:12 -0500 Subject: SL-20546: Use narrow() explicit conversion from F64 to F32. --- indra/llcommon/llrand.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llrand.cpp') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index 20e25177f0..702d6b34c9 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -93,7 +93,7 @@ inline F32 ll_internal_random() // Per Monty, it's important to clamp using the correct fmodf() rather // than expanding to F64 for fmod() and then truncating back to F32. Prior // to this change, we were getting sporadic ll_frand() == 1.0 results. - F32 rv{ ll_internal_random_unclamped() }; + F32 rv{ narrow(ll_internal_random_unclamped()) }; if(!((rv >= 0.0) && (rv < 1.0))) return fmodf(rv, 1.0f); return rv; } -- cgit v1.2.3 From e7ae20c96fccdad06e39a3f8e5fe61a812029242 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 17 Nov 2023 10:24:14 -0500 Subject: SL-20546: Avoid promoting F32 to double just to compare bounds. --- indra/llcommon/llrand.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llrand.cpp') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index 702d6b34c9..8206bf8e0c 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -94,7 +94,7 @@ inline F32 ll_internal_random() // than expanding to F64 for fmod() and then truncating back to F32. Prior // to this change, we were getting sporadic ll_frand() == 1.0 results. F32 rv{ narrow(ll_internal_random_unclamped()) }; - if(!((rv >= 0.0) && (rv < 1.0))) return fmodf(rv, 1.0f); + if(!((rv >= 0.0f) && (rv < 1.0f))) return fmodf(rv, 1.0f); return rv; } -- cgit v1.2.3 From 5fa7f69101a889009194eeddb927599d7536613f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 17 Nov 2023 14:31:21 -0500 Subject: SL-20546: Defend llrand's random generator against concurrent access by making it thread_local. --- indra/llcommon/llrand.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) (limited to 'indra/llcommon/llrand.cpp') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index 8206bf8e0c..e4065e23bf 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -28,7 +28,6 @@ #include "llrand.h" #include "lluuid.h" -#include "mutex.h" /** * Through analysis, we have decided that we want to take values which @@ -59,16 +58,9 @@ * to restore uniform distribution. */ -static std::mutex gRandomGeneratorMutex; -static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); - -inline F64 ll_internal_random_unclamped() -{ - // gRandomGenerator is a stateful static object, which is therefore not - // inherently thread-safe. Lock it before use. - std::unique_lock lk(gRandomGeneratorMutex); - return gRandomGenerator(); -} +// gRandomGenerator is a stateful static object, which is therefore not +// inherently thread-safe. +static thread_local LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); // no default implementation, only specific F64 and F32 specializations template @@ -81,7 +73,7 @@ inline F64 ll_internal_random() // CPUs (or at least multi-threaded processes) seem to // occasionally give an obviously incorrect random number -- like // 5^15 or something. Sooooo, clamp it as described above. - F64 rv{ ll_internal_random_unclamped() }; + F64 rv{ gRandomGenerator() }; if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } @@ -93,7 +85,7 @@ inline F32 ll_internal_random() // Per Monty, it's important to clamp using the correct fmodf() rather // than expanding to F64 for fmod() and then truncating back to F32. Prior // to this change, we were getting sporadic ll_frand() == 1.0 results. - F32 rv{ narrow(ll_internal_random_unclamped()) }; + F32 rv{ narrow(gRandomGenerator()) }; if(!((rv >= 0.0f) && (rv < 1.0f))) return fmodf(rv, 1.0f); return rv; } -- cgit v1.2.3 From a2552a555669490dc2ca173a48989d1b30e62c56 Mon Sep 17 00:00:00 2001 From: Alexander Gavriliuk Date: Thu, 8 Feb 2024 21:03:59 +0100 Subject: Build fix for Visual Studio patch --- indra/llcommon/llrand.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llrand.cpp') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index e4065e23bf..0192111574 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -85,7 +85,7 @@ inline F32 ll_internal_random() // Per Monty, it's important to clamp using the correct fmodf() rather // than expanding to F64 for fmod() and then truncating back to F32. Prior // to this change, we were getting sporadic ll_frand() == 1.0 results. - F32 rv{ narrow(gRandomGenerator()) }; + F32 rv{ narrow(gRandomGenerator()) }; if(!((rv >= 0.0f) && (rv < 1.0f))) return fmodf(rv, 1.0f); return rv; } -- cgit v1.2.3