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

Small optimizations on iggen buffer handling #317

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Conversation

fknorr
Copy link
Contributor

@fknorr fknorr commented Dec 4, 2024

perform_task_buffer_accesses updates last-writers twice to gracefully handle overlapping writes, which is an edge case. This PR quickly checks if overlapping writes are present, and sticks to a single update if there are not. By transposing the loop nest from chunk -> bid to bid -> chunk, we can also save avoid constructing another unordered_map.

Results are not looking too impressive in the benchmark report, but I do get a consistent 4% speedup for RSim room_small, which is scheduler bound on gpuc3.

@fknorr fknorr added this to the 0.7.0 milestone Dec 4, 2024
@fknorr fknorr requested review from psalz, PeterTh and GagaLP December 4, 2024 11:20
@fknorr fknorr self-assigned this Dec 4, 2024
@celerity celerity deleted a comment from github-actions bot Dec 4, 2024
@fknorr fknorr force-pushed the iggen-buffer-opt branch 2 times, most recently from 0bd705c to a46a214 Compare December 4, 2024 11:24
Copy link

github-actions bot commented Dec 4, 2024

Check-perf-impact results: (c8fb992b35322012b54e351345fdf71a)

✔️ No significant performance change in the microbenchmark set. You are good to go!

Relative execution time per category: (mean of relative medians)

  • command-graph : 1.01x
  • graph-nodes : 1.03x
  • grid : 1.01x
  • instruction-graph : 0.97x
  • scheduler : 0.98x
  • system : 0.98x
  • task-graph : 1.02x

@coveralls
Copy link

coveralls commented Dec 4, 2024

Pull Request Test Coverage Report for Build 12787152826

Details

  • 36 of 36 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 95.067%

Totals Coverage Status
Change from base Build 12390656669: 0.02%
Covered Lines: 7087
Relevant Lines: 7192

💛 - Coveralls

Copy link
Contributor

@GagaLP GagaLP left a comment

Choose a reason for hiding this comment

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

Nicely done.
LGTM! 👍

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.

LGTM! I've suggested two comment changes that I've added for my understanding while investigating how to implement replicated writes!

src/instruction_graph_generator.cc Show resolved Hide resolved
src/instruction_graph_generator.cc Show resolved Hide resolved
@fknorr fknorr force-pushed the iggen-buffer-opt branch 2 times, most recently from 1ee0a18 to 3be3378 Compare January 15, 2025 11:15
@fknorr fknorr merged commit 277403a into master Jan 15, 2025
32 checks passed
@fknorr fknorr deleted the iggen-buffer-opt branch January 15, 2025 13:22
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.

4 participants