Skip to content

add_thunk: Optimize domination check #342

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

Merged
merged 4 commits into from
Apr 15, 2022
Merged

Conversation

jpsamaroo
Copy link
Member

dominates (called in add_thunk! -> register_future!) previously used a duplicative recursive implementation, which has known issues. This switches dominates to a visitor pattern, which results in far better performance on large task trees.

Fixes #341

@jpsamaroo
Copy link
Member Author

@DrChainsaw the "Remove workers" test is being flaky again, so I'm disabling it for now. I think we should sit down and rethink those tests (and potentially Dagger's own add/remove interfaces) to ensure that we can reliably test that behavior, regardless of what decisions the scheduler makes.

@DrChainsaw
Copy link
Contributor

"Remove workers" test is being flaky again, so I'm disabling it for now.

Yeah, I guess it was only a matter of time. I should probably have used better judgement and not tried to push that "fix" :)

Perhaps just let it be untested until work stealing is implemented as the add/remove functionality anyways is a bit pointless without it?

@jpsamaroo jpsamaroo force-pushed the jps/faster-dominates branch from 02f0fae to b024f1f Compare April 14, 2022 21:05
@jpsamaroo jpsamaroo force-pushed the jps/faster-dominates branch from df55d47 to 6600bce Compare April 15, 2022 16:25
@jpsamaroo jpsamaroo merged commit 04c7c42 into master Apr 15, 2022
@jpsamaroo jpsamaroo deleted the jps/faster-dominates branch April 15, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Execution slow for a simple sequence
2 participants