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

feat: Better thread local access #15

Merged
merged 4 commits into from
Feb 21, 2024
Merged

Conversation

Bluefinger
Copy link
Owner

The original implementation of ThreadLocalEntropy was based on rand's ThreadRng, making a few changes to not require reseeding or forking protection in order to assume cases more in-line with game application usage. However, as a result, it picked up the issues with the original implementation, such as leaking memory if the destructors for ThreadLocalEntropy didn't run.

This new implementation does away with cloning an Rc reference, instead opting for keeping the references within the LocalKey::with closure, avoiding any reference counting. The Rc prevents premature freeing during thread-local destructors, while keeping the ref inside the with closure ensures that we never end up in a situation where we might have a ref counted instance that never gets cleaned up.

This also apparently allows the compiler to make better optimisations with the thread local access. Normal release profile has the perf for the old implementation and new implementation around the same, with the new implementation slightly ahead for the worst case scenario and with better mean/median times. With codegen-units = 1 and LTO, it optimises considerably better.

NOTE: This is tuned for the specific usage of ThreadLocalEntropy within bevy_rand. For one time accesses with ThreadLocalEntropy that then use the RNG source before dropping, this implementation is much faster. For accesses that hold the reference for longer to be used repeatedly before dropping, the original ThreadRng implementation is faster. bevy_rand's case is more on the one-time accesses.

@Bluefinger Bluefinger added the enhancement New feature or request label Feb 20, 2024
@Bluefinger Bluefinger force-pushed the optimise-thread-local-entropy branch from ba91c15 to 46f2d4d Compare February 21, 2024 09:27
@Bluefinger Bluefinger merged commit 5975a8c into main Feb 21, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant