[release/9.0-rc2] Fix Random.GetItems breaking change in produced sequence #108018
+6
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Developers, often in tests, rely on seeded Random instances producing the same sequence of values on every use. We made a change in .NET 9, though, that changed the sequence GetItems produces, due to employing a different algorithm. This fixes that special-case to only be used when the developer couldn't rely on the results being deterministic, namely when using either `new Random()` or `Random.Shared`. If a seed is provided or if a custom derived implementation is used, it falls back to the old behavior.
dotnet-issue-labeler
bot
added
the
needs-area-label
stephentoub
added
area-System.Runtime
and removed
needs-area-label
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
This was referenced Sep 19, 2024
stephentoub
changed the title
rbhanda
added
Servicing-approved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport of #108017 to release/9.0-rc2
/cc @stephentoub
Customer Impact
#92229 added a special-case to optimize Random.GetItems in certain common situations. That, however, ends up observably changing the sequence of values the Random instance produces. If the Random instance was just constructed as
new Random()orRandom.Shared, there's no reasonable way for a developer to have taken a dependency on the sequence. But if the instance was constructed with a seed, e.g.new Random(42), or if a custom derived type was used, the change is observable, yielding a different sequence of values than previously was produced. Such seeded instances are typically used when it's important that the same sequence always be produced, such as in tests. This fix adds an additional guard clause so that the optimization only kicks in for the cases where it's not observable.Regression
#92229
Testing
Validated that the same sequence produced before is now produced, and also that .NET 8 performance isn't regressed.
Risk
Low. It just means that we no longer employ the .NET 9 optimization in some cases we previously employed it.