Experiment: Replace global mutable variables for random number generators #1
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This is an experiment how a different usage of random number generators could improve CPU performance. The result is that
simple_multi_job_100_benchmark
takes 15% less time on an Apple M1 machine. This PR is not meant to be merged, but just an experiment.Motivation
To see potential performance issues, I profiled CPU usage of
simple_multi_job_100_benchmark
(The description inCriterion
says "a problem with 50 multi jobs". This was done with the Mac applicationInstruments
. Noticeable are calls to the random number generator which take a substantial percentage of the whole time.Analysis
REPEATABLE_RNG
andRANDOMIZED_RNG
are global / thread local mutable variables. They are used by a non mutableArc<dyn Random>
.There are several performance costs connected to that, mainly:
if
to change between repeatable and non-deterministic behavior.Idea
Use a new struct
PureRandom
which encapsulated the random number generator and pass it directly without smart pointers to each function. This way Rust can use zero cost abstractions handling it. Also our functions become pure functions: If identically seeded RNGs are given as arguments, the return value will always be the same. This can also be helpful for deterministic unit and performance tests.Implementation
Not every usage of
Random
was replaced, only that inLegSelection
. So most ofvrp
still uses the previous implementation. NowLegSelection
needs to be mutable, so how this is handled in someContexts
had to be changed too - non mutable references don't work anymore here.When used together with
Rayon
, each iteration needs a differentRNG
, so we need to create all of them before iteration. Each newDefaultPureRandom
instance is different from each other, still everything is deterministic.Performance testing
In
general_benchmarks
I've changed the sample size from 15 to 100 to get more reliable results. The time estimate forsimple_multi_job_100_benchmark
changed from 2.3671 seconds to 1.9978 s. This is roughly 15% less time or from the perspective: Before the change roughly 18% more time was needed.Issues / Todos:
LegSelection
the right place to containPureRandom
? Maybe remove it from there so thatStochastic
doesn't have any values. This way we could change thecontexts
to contain only non mutable references again. ThePureRandom
would then be passed to each function as a separate value - this would also work in other cases where randomness is needed butLegSelection
is not used.FakeRandom
for unit tests? Maybe deterministic behaviour is enough, otherwise we might want to make some structs and functions generic instead of using dynamic dispatch.