Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<random>: generate_canonical() could avoid calling log2() at runtime #1964

Closed
StephanTLavavej opened this issue Jun 8, 2021 · 1 comment · Fixed by #2498
Closed

<random>: generate_canonical() could avoid calling log2() at runtime #1964

StephanTLavavej opened this issue Jun 8, 2021 · 1 comment · Fixed by #2498
Labels
fixed Something works now, yay! performance Must go faster

Comments

@StephanTLavavej
Copy link
Member

Gautham Beeraka reported a performance issue in generate_canonical(), where it calls log2():

STL/stl/inc/random

Lines 248 to 259 in 4c862ee

template <class _Real, size_t _Bits, class _Gen>
_NODISCARD _Real generate_canonical(_Gen& _Gx) { // build a floating-point value from random sequence
_RNG_REQUIRE_REALTYPE(generate_canonical, _Real);
const size_t _Digits = static_cast<size_t>(numeric_limits<_Real>::digits);
const size_t _Minbits = _Digits < _Bits ? _Digits : _Bits;
const _Real _Gxmin = static_cast<_Real>((_Gx.min)());
const _Real _Gxmax = static_cast<_Real>((_Gx.max)());
const _Real _Rx = (_Gxmax - _Gxmin) + _Real{1};
const int _Ceil = static_cast<int>(_STD ceil(static_cast<_Real>(_Minbits) / _STD log2(_Rx)));

For engines like mt19937 and mt19937_64, this is a really expensive way of computing 32.0 and 64.0.

Solving this in a completely general manner might be somewhat difficult, but perhaps we could simply special-case the Standard engines that are known to have a power-of-2 range.

@StephanTLavavej StephanTLavavej added the performance Must go faster label Jun 8, 2021
@pjessesco
Copy link
Contributor

Hi, if I understand this issue correctly, is the suggested solution comparing _Gen with standard engines and specifing the denominator manually?

For example,

if(_Gen == mt19937) 
    denominator = 32
else if(_Gen == other pre-defined engines)
    denominator = something
....
else
    denominator = _STD log2(_Rx)

CaseyCarter added a commit to CaseyCarter/STL that referenced this issue Dec 31, 2021
... to calculate parameters at compile-time for random engines with power-of-two range. This greatly improves debug codegen, and release codegen for compilers that do not constant fold `std::ceil` and/or `std::log2`.

Fixes microsoft#1964.
CaseyCarter added a commit to CaseyCarter/STL that referenced this issue Dec 31, 2021
... to calculate parameters at compile-time for random engines with power-of-two range. This greatly improves debug codegen, and release codegen for compilers that do not constant fold `std::ceil` and/or `std::log2`.

Fixes microsoft#1964.
CaseyCarter added a commit to CaseyCarter/STL that referenced this issue Jan 1, 2022
... to calculate parameters at compile-time for random engines with power-of-two range. This greatly improves debug codegen, and release codegen for compilers that do not constant fold `std::ceil` and/or `std::log2`.

Fixes microsoft#1964.
MattStephanson added a commit to MattStephanson/STL that referenced this issue Jan 21, 2022
...up to 64 bits of entropy. Leave the original implementation for
more bits and naughty generators (I'm looking at you, tr1) whose
min and max functions aren't static.

Fixes microsoft#1964. Alternative to microsoft#2452.
CaseyCarter pushed a commit that referenced this issue Feb 7, 2022
...up to 64 bits of entropy. Leave the original implementation for
more bits and naughty generators (I'm looking at you, tr1) whose
`min` and `max` functions aren't static.

Fixes #1964.
@CaseyCarter CaseyCarter added the fixed Something works now, yay! label Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed Something works now, yay! performance Must go faster
Projects
None yet
3 participants