Plumb statistics object into ProgressThread for all engines - #23082
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Needs rapidsai/rapidsmpf#1118 |
0400d76 to
216d366
Compare
pentschev
left a comment
There was a problem hiding this comment.
Left another comment, and I think a cuDF-level test that performs a communicator-backed operation and observes its send/receive counter through gather_statistics() would be good to have, otherwise LGTM. Thanks Lawrence!
We need this so that the communicators that we build have an active statistics object for keeping tack of communcation volume. Previously we never noticed this because the only communication volume statistics we tracked were from the Shuffler that got its statistics from the buffer resource. However, it makes more sense to use the communicator since (for example) the allreduce implementation doesn't allocate and therefore doesn't have a buffer resource.
216d366 to
1ffbde5
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a ChangesStatistics reuse and reset
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
We might still see an event-loop stat due to timing.
|
/merge |
Description
We need this so that the communicators that we build have an active statistics object for keeping tack of communcation volume.
Previously we never noticed this because the only communication volume statistics we tracked were from the Shuffler that got its statistics from the buffer resource. However, it makes more sense to use the communicator since (for example) the allreduce implementation doesn't allocate and therefore doesn't have a buffer resource.
Checklist