[
https://issues.apache.org/jira/browse/RNG-57?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16633915#comment-16633915
]
Alex D Herbert commented on RNG-57:
-----------------------------------
{quote}I'm not sure whether it is your intended contribution to the codebase or
just test code for benchmarking.
Unless I'm missing something, I think that the public API should not expose
classes with "Cache" in their name.
{quote}
This is an internal class. It implements the private interface
{{CachedUniformRandomProvider}}.
I am currently still investigating things for benchmarking.
I now have speed results from 3 machines using Java 8:
- MacOS 10.13 : Intel Core i7 2.7GHz (laptop)
- Windows 7 : Intel Core i7-2600 3.4 GHz
- Linux Ubuntu 16.04 LTS : Intel(R) Xeon(R) CPU E5-1680 v3 @ 3.20GHz
The results for the {{nextInt}} for a {{LongProvider}} are all consistent.
Caching the long and returning the upper and lower 32 bits is the best method.
The results for the {{nextBoolean}} are variable across
architectures/platforms. This makes picking one method difficult and I have no
idea if the differences are due to chip architecture, the VM on the platform or
both. However out of the candidates they are all faster than not doing it.
So it may be best to just pick one that works the same way for {{IntProvider}}
and {{LongProvider}} (for improved maintainability). Or do a more rigorous
investigation of the speed using more architectures and JVMs.
{quote}Do your benchmarks show that the cache is always useful for performance
(or to never hurt)?
If so, it will be always "on" (an implementation detail).
{quote}
Yes.
Adding a cache by default for {{nextInt}} to the {{LongProvider}} would be
useful. I think this would be a more commonly used routine, e.g. for array
randomisation and to generate {{nextFloat}}. It also has a preferred
implementation that is clear from the tests I have done.
I see {{nextBoolean}} as more specialised. But it could be added with one of
the good implementations. Or the various implementations added to
{{commons-rng-examples}}, for instance to {{examples-sampling}}.
Adding this functionality as a default would require alterations to support
{{RestorableUniformRandomProvider}}. Each provider currently creates and
restores its own state as a {{byte[]}} within methods inherited from
{{BaseProvider}}:
{code:java}
protected byte[] getStateInternal();
protected void setStateInternal(byte[] state);
{code}
The change could alter the inheritance hierarchy so that implementations that
inherit from {{LongProvider}} and {{IntProvider}} no longer implement these
methods directly. Instead they are implemented at the level of
{{Long/IntProvider}} so that the state of the cached {{nextBoolean}} and
{{nextInt}} are saved there. The providers can then implement new methods to
get the {{byte[]}} representation of their state. This is then combined with
the {{byte[]}} representation of the cached values.
Note that adding the cache for {{nextInt}} to the {{LongProvider}} would
require updating the benchmark tests shown in the [User
Guide|https://commons.apache.org/proper/commons-rng/userguide/rng.html] for
Performance and Quality. This is not something I have yet investigated so is a
big change.
> CachedUniformRandomProvider for nextBoolean() and nextInt()
> -----------------------------------------------------------
>
> Key: RNG-57
> URL: https://issues.apache.org/jira/browse/RNG-57
> Project: Commons RNG
> Issue Type: Improvement
> Components: sampling
> Affects Versions: 1.2
> Reporter: Alex D Herbert
> Priority: Minor
> Labels: performance
>
> Implement a wrapper around a {{UniformRandomProvider}} that can cache the
> underlying source of random bytes for use in the methods {{nextBoolean()}}
> and {{nextInt()}} (in the case of {{LongProvider}}). E.g.
> {code:java}
> LongProvider provider = RandomSource.create(RandomSource.SPLIT_MIX_64);
> CachedLongProvider rng = new CachedLongProvider(provider);
> // Uses cached nextLong() 64 times
> rng.nextBoolean();
> // Uses cached nextLong() twice
> rng.nextInt();
> IntProvider provider = RandomSource.create(RandomSource.KISS);
> CachedIntProvider rng2 = new CachedIntProvider(provider);
> // Uses cached nextInt() 32 times
> rng2.nextBoolean();
> // This could be wrapped by a factory method:
> UniformRandomProvider rng = CachedUniformRandomProviderFactory.wrap(
> // Any supported source: IntProvider or LongProvider
> RandomSource.create(RandomSource...));
> {code}
> The implementation should be speed tested to determine the benefit for
> {{nextBoolean()}} and if {{nextInt()}} can be improved for {{LongProviders}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
