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

scx_layered: Improve layer core growth algos #1098

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

hodgesds
Copy link
Contributor

@hodgesds hodgesds commented Dec 12, 2024

Improve layer core growth algos to be aware of the number of layers. When applying layer core growth stategies offset core iteration order by a chunked layer offset. This makes Linear, Reverse and other growth algos more useful in that when multiple layers use the same growth strategy they will not longer attempt to grow using the same core order.

Example with changing the default stress-ng layer to Sticky:
image

@hodgesds hodgesds force-pushed the layered-growth-algo-refactor branch 2 times, most recently from e667bd0 to 5c9533d Compare December 12, 2024 15:42
fn layer_offset(&self) -> usize {
self.layer_specs
.iter()
.position(|spec| spec.name == *self.spec.name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think name is guaranteed to be unique. How is this different from .layer_idx?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, layer_idx is probably better. I wish it was a field on the LayerSpec.

fn rotate_layer_offset(&self, vec: &'a mut Vec<usize>) -> &Vec<usize> {
let offset = self.layer_offset();
let num_cores = self.topo.all_cores.len();
let chunk = (num_cores as f64 / self.layer_specs.len() as f64).ceil() as usize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.div_ceil()?

Improve layer core growth algos to be aware of the number of layers.
When applying layer core growth stategies offset core iteration order by
a chunked layer offset.

Signed-off-by: Daniel Hodges <hodges.daniel.scott@gmail.com>
@hodgesds hodgesds force-pushed the layered-growth-algo-refactor branch from 5c9533d to 2536788 Compare December 12, 2024 16:38
@hodgesds hodgesds enabled auto-merge December 12, 2024 16:39
@hodgesds hodgesds added this pull request to the merge queue Dec 12, 2024
Copy link
Contributor

@JakeHillion JakeHillion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I'm hoping with this we can write some growth algorithms that have good avoidance while also spreading out effectively.

Merged via the queue into sched-ext:main with commit aa7b375 Dec 12, 2024
23 checks passed
@hodgesds hodgesds deleted the layered-growth-algo-refactor branch December 12, 2024 16:53
@htejun
Copy link
Contributor

htejun commented Dec 12, 2024

BTW, I don't think this is a viable long term direction. This is useful for quickly experimenting with different ordering but this approach can't reliably achieve an outcome that we ultimately want. Imagine a scenario where three layers are changing their sizes dynamically.

Let's say there are 4 llcs with 4 cpus for simplicity. Let's say the three layers start out sized 8, 3, 5. The ideal allocation would be two llcs for the first one, one full llc for the third one and the rest in the remaining llc. Let's say the sizes shift and we end up with 5, 3, 8 and stabilizes. There is no way to achieve the ideal allocaiton without actively moving the allocation for the middle layer.

Even ignoring the need for dynamic reallocation, placing the layers at equidistances is unlikely to bring the allocation close to the ideal state given how they tend to differ widly in sizes. The big guys will overrun the positions for the smaller guys who then in turn will encroach into other layers' slots.

@hodgesds
Copy link
Contributor Author

Yeah, having the ideal growth be something that is dynamic is definitely a better solution. This is just a short term hack to make it a little better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants