search

celerity / celerity-runtime

High-level C++ for Accelerator Clusters
https://celerity.github.io
MIT License
139 stars 18 forks source link

Fix: Do not value-capture buffers into host tasks in system_benchmarks #264

Closed fknorr closed 1 month ago

fknorr commented 1 month ago

It is illegal to value-capture buffers etc into host task lambdas, and actually breaks with the new runtime. Two instances of this pattern, both in system_benchmarks.cc, were missed in #244.

github-actions[bot] commented 1 month ago

Check-perf-impact results: (7b849de16ff11660b98988ab0b032db7)

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

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 9997398762

Details

Totals Coverage Status
Change from base Build 9992874606: 0.03%
Covered Lines: 7231
Relevant Lines: 7505
💛 - Coveralls
fknorr commented 1 month ago

Currently (buffer-manager runtime) this just works because lifetime extension makes sure that the final reference is dropped on TDAG pruning. The IDAG runtime detects the attempt to destroy a buffer from the wrong thread (scheduler thread, which prunes the IDAG) and aborts, but for some reason the most recent benchmark run timed out instead of aborting. Not sure why that was, but it worked on my machine.