-
Notifications
You must be signed in to change notification settings - Fork 87
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
scx_layered: Consume from local LLCs for dispatch #919
Conversation
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.
LGTM
Some stress results, not 100% sure if the
PR:
|
When dispatching consume from DSQs in the local LLC first before trying remote DSQs. This should still be fair as the layer iteration order will be maintained. Signed-off-by: Daniel Hodges <[email protected]>
dea4451
to
775d09a
Compare
More tests on a partially saturated machine (80 instances, 176 CPUs)
PR:
These tests were run with the following config:
|
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.
As discussed offline I'm not sure these test results are consistent, and I had some issues running them for longer on main. However the change makes sense!
b55c1bc
to
3417727
Compare
Signed-off-by: Daniel Hodges <[email protected]>
3417727
to
4fc0509
Compare
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.
NACK to the flag without good reason. Normally I want flags for most changes, but in this case the change makes logical sense and doesn't significantly change our dispatching. The current implication of duplicating the entire loops is not good for code readability/maintenance at all.
If the flag is necessary (we can't settle on a single behaviour), can we implement it by doing divmod in the loop instead? This gives the same optionality without duplicating the bodies.
I mostly want the flag for doing more testing on actual workloads. It's bothersome to have to rebuild different versions of the scheduler to test the differences in performance and synthetic benchmarks only go so far in testing. |
Alternatively, could you pull out the body from both loops into a static inline function and then have the |
Agreed, fan of more flags. It's that or more bisect/patch etc. to debug issues. |
When dispatching consume from DSQs in the local LLC first before trying remote DSQs. This should still be fair as the layer iteration order will be maintained.