-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
runtime: only mitigate false sharing for multi-threaded runtimes #6240
base: master
Are you sure you want to change the base?
runtime: only mitigate false sharing for multi-threaded runtimes #6240
Conversation
Regarding benchmarks, the largest ones I got are these:
However, this PR really shouldn't change the multi-threaded runtime, so I'm not sure what's going on. Click me to view benchmark results
|
A 2000% perf regression isn't nothing. Given that this is happening in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should dig into the benchmark regression first.
(leaving this as a review to block merging)
The benchmarks are nonsense. See #6243 for the fix. New benchmarks on their way ... |
In #5809, we padded the task struct to mitigate false sharing when different worker threads poll different tasks. However, for runtimes with only a single worker thread, this doesn't make sense. This patch makes it so that we don't require a high alignment for runtimes without multiple threads, where padding the task struct makes it consume more memory for no gain.
This updates
src/util/cacheline.rs
to match what the task struct was annotated with. I don't know why they were different.