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

Implement breadth-triggered Horizons #199

Merged
merged 5 commits into from
Sep 12, 2023
Merged

Implement breadth-triggered Horizons #199

merged 5 commits into from
Sep 12, 2023

Conversation

PeterTh
Copy link
Contributor

@PeterTh PeterTh commented Aug 3, 2023

This implements Horizons which are triggered when the breadth of the execution front exceeds a given limit, which can be configured at runtime (currently defaults to 64).

This also makes it impossible to run out of task slots in normal usage (i.e. unless the user manually sets the limit to a value that exceeds the task ring buffer capacity).

This PR includes #196 and #197 which should be merged first, and is actually rather small by itself.
I will rebase it once those are merged.

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Check-perf-impact results: (9e900a306dc9f17e4a27439205a7680c)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

fknorr pushed a commit to fknorr/celerity-runtime that referenced this pull request Aug 23, 2023
Copy link
Contributor

@fknorr fknorr 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've been looking forward to this.

A few requests nonetheless.

include/host_utils.h Show resolved Hide resolved
include/intrusive_graph.h Outdated Show resolved Hide resolved
include/ranges.h Outdated Show resolved Hide resolved
examples/independent_tasks/independent_tasks.cc Outdated Show resolved Hide resolved
@PeterTh PeterTh force-pushed the petert/indep-tasks branch from 9ff3b29 to ba0afea Compare August 31, 2023 14:43
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

test/system_benchmarks.cc Show resolved Hide resolved
test/task_graph_tests.cc Outdated Show resolved Hide resolved
test/task_graph_tests.cc Outdated Show resolved Hide resolved
test/task_graph_tests.cc Outdated Show resolved Hide resolved
test/task_graph_tests.cc Outdated Show resolved Hide resolved
test/task_graph_tests.cc Outdated Show resolved Hide resolved
test/task_graph_tests.cc Outdated Show resolved Hide resolved
test/test_utils.h Outdated Show resolved Hide resolved
@PeterTh
Copy link
Contributor Author

PeterTh commented Sep 1, 2023

This should now be ready for re-review.

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Check-perf-impact results: (7a518c63eb0655471fea6a4902fc0af7)

⚠️ Significant slowdown in some microbenchmark results: 7 individual benchmarks affected
🚀 Significant speedup in some microbenchmark results: 9 individual benchmarks affected
Added microbenchmark(s): benchmark independent task pattern with N tasks - 100 / task generation, benchmark independent task pattern with N tasks - 1000 / task generation, benchmark independent task pattern with N tasks - 5000 / task generation

Overall relative execution time: 1.00x (mean of relative medians)

Copy link
Member

@psalz psalz left a comment

Choose a reason for hiding this comment

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

Thanks, I've added a few minor notes!

test/task_ring_buffer_tests.cc Outdated Show resolved Hide resolved
include/ranges.h Show resolved Hide resolved
test/system_benchmarks.cc Show resolved Hide resolved
include/ranges.h Show resolved Hide resolved
include/task_manager.h Show resolved Hide resolved
test/range_tests.cc Outdated Show resolved Hide resolved
include/host_utils.h Show resolved Hide resolved
include/host_utils.h Show resolved Hide resolved
@fknorr fknorr added this to the 0.5.0 milestone Sep 8, 2023
@PeterTh PeterTh merged commit f190da3 into master Sep 12, 2023
@PeterTh PeterTh deleted the petert/indep-tasks branch September 12, 2023 09:42
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