Remove task_manager / buffer_manager dependency from recorders

celerity / celerity-runtime

High-level C++ for Accelerator Clusters

https://celerity.github.io

MIT License

139 stars 18 forks source link

Remove task_manager / buffer_manager dependency from recorders #216

Closed fknorr closed 9 months ago

fknorr commented 11 months ago

This PR is a clean-up refactoring for parts of #197, and A back-port from the instruction graph branch which will remove buffer_manager and extend the recording / printing infrastructure further.

Adds buffer_recorder for buffer-debug-name tracking (so we don't have to ask buffer_manager)
Passes buffer_recorder into print_task_graph / print_command_graph, and task_recorder into print_command_graph, thereby removing duplicate information from records

github-actions[bot] commented 11 months ago

Check-perf-impact results: (5a19ced85f862a00d0114dd241122462)

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

PeterTh commented 11 months ago

Yeah, we specifically discussed the buffer naming lifetime concern when I was building this, which is the reason for the design. I generally prefer that the information in the record is as self-contained as possible once it is recorded, and doesn't have dependencies on other data sources.

fknorr commented 9 months ago

Thanks for the feedback - we're not doing any of this after all, instead, all DAG generators will track debug names and copy them into every record. One of the future IDAG PRs will address this.