Convert Tracker structs to columnar#762
Merged
frankmcsherry merged 2 commits intoTimelyDataflow:masterfrom Mar 13, 2026
Merged
Conversation
antiguru
approved these changes
Mar 13, 2026
Member
antiguru
left a comment
There was a problem hiding this comment.
Seems alright, but I wonder if we could push it further to flatten even more. We already have to pay for an increase in complexity, so why stop here?
Comment on lines
+374
to
+379
| /// Internal operator connectivity, columnar form of `Vec<Vec<PortConnectivity<T::Summary>>>`. | ||
| /// Indexed by `(node, input_port)` to yield `(output_port, summary)` pairs. | ||
| nodes: Vecs<Vecs<Vec<(usize, T::Summary)>>>, | ||
| /// Edge connectivity, columnar form of `Vec<Vec<Vec<Target>>>`. | ||
| /// Indexed by `(node, output_port)` to yield target slices. | ||
| edges: Vecs<Vecs<Vec<Target>>>, |
Member
There was a problem hiding this comment.
Since we're flattening this .. what about flattening it further? Instead of nested vecs, have the usual index vec + flat vector, one for each level. I can imagine it'll get unwieldy for no benefit, but maybe worth exploring.
Member
Author
There was a problem hiding this comment.
Vecs is what you are thinking of! :D
Member
Author
There was a problem hiding this comment.
It could maybe be clearer as <Vec<Vec<PortConnectivity<T::Summary>>> as Columnar>::Container, but you can't quite get there yet (w/o T::Summary: Columnar>).
376b43e to
1311dbb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is largely authored by Claude, as a way to convert the types in the reachability tracker to have columnar layouts, rather than deeply nested short vectors and allocations. The types are all dense integer indexed (nodes, ports), and standard columnar vector layouts work great here.
The win is that at runtime there is less memory to hop through to reach path summaries, among other things. There is a bonus benefit that large graphs will not take as long terminating; the deallocation has taken seconds on large benchmarks.