Feature #17322
closedDeprecate `Random::DEFAULT` and introduce `Random.default()` method to provide Ractor-supported default random generator
Description
Random::DEFAULT
a default random generator used by rand
, srand
, Array#shuffle
without a given random generator, and so on.
Random generators are not thread-safe, so they are not ractor safe, and they are not shareable.
So a program refer to Random::DEFAULT
on non-main ractor, it causes an error.
To provide per-ractor default random generator, this ticket propose the Random.default()
method which returns per-ractor random generator.
Random::DEFAULT
is a result of Random.default()
on main-ractor and it should be deprecated, or at least it should not be used on multi-ractor supporting apps and libraries.
Updated by shyouhei (Shyouhei Urabe) almost 4 years ago
Why not introduce general-purpose per-ractor variables first, instead of magical method that interacts with the crrent ractor behind-the-scene.
Updated by ko1 (Koichi Sasada) almost 4 years ago
Why not introduce general-purpose per-ractor variables first, instead of magical method that interacts with the crrent ractor behind-the-scene.
Do you mean new syntax or method?
Just now i'm writing the per-ractor storage class (not a syntax).
However, it needs new constant name and method access, such as Random::DEFAULT2.value
, and Random.default
seems better, so I filed this ticket first.
Updated by shyouhei (Shyouhei Urabe) almost 4 years ago
ko1 (Koichi Sasada) wrote in #note-2:
Why not introduce general-purpose per-ractor variables first, instead of magical method that interacts with the crrent ractor behind-the-scene.
Do you mean new syntax or method?
Both :)
But I understand that new syntax needs time (or maybe it's matz who needs time). A method would be a good starting point. Maybe Ractor#[]
(like Thread#[]
) ?
Updated by ko1 (Koichi Sasada) almost 4 years ago
Maybe Ractor#[] (like Thread#[]) ?
yes, it is one option (but I don't like this, make a new ticket soon about it with reasons).
However, if we choose Ractor#[]
, Ractor.current[:DEFAULT_RANDOM]
seems not good idea (not impossible).
Updated by shyouhei (Shyouhei Urabe) almost 4 years ago
It sounds rather intuitive to me that rand
etc. are not sharable among ractors. It is their nature to mutate process-global state of executions.
If a programmer wants randomness inside of a ractor they needs special care to separate their random source from the rest of the world. This means API changes are inevitable. I'm +1 to the deprecation part.
Updated by Eregon (Benoit Daloze) almost 4 years ago
In JRuby and in TruffleRuby, Random instances are thread-safe (i.e., they use synchronization internally).
Also, rand
, etc, use a per-Thread Random instance to avoid needless contention when calling rand
in multiple threads concurrently.
So, I agree we should deprecate Random::DEFAULT
.
I think rand
, etc should use a thread-local Random
instance (and thread-local implies Ractor-local, of course).
Those thread-local Random instances should probably not be exposed to the Ruby level, so that way there is never a need to synchronize access to it.
So, I think we should deprecate Random::DEFAULT
without replacement.
Is there any case where it's useful?
Updated by Dan0042 (Daniel DeLorme) almost 4 years ago
Eregon (Benoit Daloze) wrote in #note-6:
I think
rand
, etc should use a thread-localRandom
instance (and thread-local implies Ractor-local, of course).
Those thread-local Random instances should probably not be exposed to the Ruby level, so that way there is never a need to synchronize access to it.
Bypassing the need for synchronization is a very good idea.
Is there any case where it's useful?
Good question. Found this. Doesn't look so useful.
faker-2.2.2/lib/faker.rb
35: @random || Random::DEFAULT
sentry-raven-2.11.1/lib/raven/configuration.rb
445: if Random::DEFAULT.rand >= sample_rate
That said, even though using Random::DEFAULT
seems like an anti-pattern, there's no real benefit to removing it. Maybe just convert it to an instance not used by anything? (except gems/apps)
Updated by ko1 (Koichi Sasada) almost 4 years ago
At the last dev-meeting, there is an idea to replace Random::DEFAULT
with an object which delegates operations to the per-ractor random generator.
Fortunately, Random
class object can be a good placeholder because it already has Random.rand
, Random.bytes
and so on.
Random::DEFAULT = Random
p Random::DEFAULT.rand(2) #=> 0 or 1
The specification changes are:
-
Random::DEFAULT
isn'tRandom
instance, butRandom
class object. -
Random.rand()
,Random.bytes()
and so on use per-ractor random generator. Maybe this is implementation dependent. JRuby and so on can use thread-local random generators. -
rand()
,srand()
useRandom.rand()
,Random.srand()
respectively (but it doesn't traceRandom::DEFAULT
replacement as current implementation doesn't) -
Random.seed()
is added to return current seed of per-ractor default random generator.
Advantages:
- We don't need to change most of code.
- We can introduce per-ractor (per-thread and so on) semantics naturally.
Disadvantages => It can break the compatibility because Random::DEFAULT
is a Class
instance.
- can not save the state of
Random::DEFAULT
- We can not use
dup
for it (there is a test usingRandom::DEFAULT.dup
in test/ruby/test_random.rb). - We can not use marshal protocol to save the random generator states of
Random::DEFAULT
. - I'm not sure how it is important.
- We can not use
- can not use instance methods of
Random
class- If user defines
Random#foo
, butRandom::DEFAULT.foo
is not available.
- If user defines
I checked Random::DEFAULT
usage with gem-codesearch and most of case it uses Random::DEFAULT.rand()
and so on.
I can't trace how to use Random::DEFAULT
for the following 4 cases.
/srv/gems/kmat-0.0.3/lib/kmat/random.rb: Random::DEFAULT.randn(*args)
/srv/gems/kmat-0.0.3/lib/kmat/random.rb: Random::DEFAULT.randn(*args)
# I can't find the `Random#randn` definition.
/srv/gems/prop_check-0.14.1/lib/prop_check/property.rb: rng = Random::DEFAULT
# I can't find how to use `rng`
/srv/gems/util-0.4.0/lib/util/args.rb: Random => Random::DEFAULT,
# I can't find how to use this information.
This is implementation: https://github.com/ruby/ruby/pull/3813
Updated by matz (Yukihiro Matsumoto) almost 4 years ago
I vote for replacing Random::DEFAULT
with ractor-safe object (Random
class). We don't need Random.default()
then.
Matz.
Updated by Eregon (Benoit Daloze) almost 4 years ago
Sounds great, except:
ko1 (Koichi Sasada) wrote in #note-8:
Random.seed()
is added to return current seed of per-ractor default random generator.
Is this needed? It sounds very bad practice from a security point of view to ever be able to read the seed directly
(without also changing it at the same time like srand
).
Updated by Eregon (Benoit Daloze) almost 4 years ago
I see, it's to be compatible for Random::DEFAULT.seed
, unfortunate.
I think we should deprecate the Random::DEFAULT
constant, it doesn't make sense anymore and it's longer than using methods on Random or Kernel directly.
Also, people might expect it to be global.
Updated by shyouhei (Shyouhei Urabe) almost 4 years ago
Despite the title of this issue, ko1’s intention is to make Ractor usable, not to make Random usable.
I think there still is a room to depreciate Random::DEFAULT. But if you want that I guess you can have a separate ticket. I guess that should be smoother than discussing it here.
Updated by nobu (Nobuyoshi Nakada) almost 4 years ago
Eregon (Benoit Daloze) wrote in #note-10:
Is this needed? It sounds very bad practice from a security point of view to ever be able to read the seed directly
(without also changing it at the same time likesrand
).
As you must not use the default RNG for security purpose, it doesn't matter.
Updated by ko1 (Koichi Sasada) almost 4 years ago
- Status changed from Open to Closed
Applied in changeset git|2db2fb9f6c742d5bd0019ccd11c7a375e1b12c0b.
per-ractor Random::DEFAULT
Random generators are not Ractor-safe, so we need to prepare
per-ractor default random genearators. This patch set
Random::DEFAULT = Randm
(not a Random instance, but the Random
class) and singleton methods like Random.rand()
use a per-ractor
random generator.
[Feature #17322]
Updated by Eregon (Benoit Daloze) almost 4 years ago
- Related to Feature #17351: Deprecate Random::DEFAULT added