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

Improve parallel #48

Merged
merged 5 commits into from
Jan 17, 2025
Merged

Improve parallel #48

merged 5 commits into from
Jan 17, 2025

Conversation

rikhuijzer
Copy link
Owner

@rikhuijzer rikhuijzer commented Jan 17, 2025

This improves upon #45 by enabling parallel processing only for the top-level call. Any nested calls use single-threaded. Time is now 22.2 ms so about a 7% improvement.

The problem with the old/current approach is that the par_iter starts but then is called another time below. Much better could be to gather all ops together and then call par_iter once. See https://fasterthanli.me/articles/recursive-iterators-rust and https://blog.danieljanus.pl/2023/07/20/iterating-trees/.

I've tried an iterator like

/// Return an iterator over `op` and all (indirect) children of `op`.
pub fn ops(op: Shared<dyn Op>) -> Box<dyn Iterator<Item = Shared<dyn Op>>> {
    let start = vec![op.clone()].into_iter();

    fn block_ops(block: Shared<Block>) -> Box<dyn Iterator<Item = Shared<dyn Op>>> {
        Box::new(
            block
                .rd()
                .ops()
                .rd()
                .clone()
                .into_iter()
                .flat_map(|op| ops(op)),
        )
    }
    if let Some(region) = op.rd().region() {
        let rest = region.rd().blocks().into_iter().flat_map(block_ops);
        Box::new(start.chain(rest))
    } else {
        Box::new(start)
    }
}

but since it is a Box, rayon needs it collected first so then just recursively calling the apply_rewrite function seems to not be that bad.

We have to collect the iterator before being able to call `par_iter` so
then all the performance gains are gone.
This reverts commit dea2ba2.
@rikhuijzer rikhuijzer merged commit baca315 into main Jan 17, 2025
7 checks passed
@rikhuijzer rikhuijzer deleted the rh/improve-parallel branch January 17, 2025 13:27
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.

1 participant