Shared reference-counted operator path #582
Conversation
Encapsulate the path for an operator as a `Rc<[usize]>` that can be shared among all interested parties, instead of passing owned vectors around. This avoids some memory allocations for rendering dataflow graphs. Signed-off-by: Moritz Hoffmann <[email protected]>
Signed-off-by: Moritz Hoffmann <[email protected]>
| @@ -94,20 +94,19 @@ where | |||
| self.edge_stash.push((source, target)); | |||
| } | |||
|
|
|||
| /// Creates a new Subgraph from a channel allocator and "descriptive" indices. | |||
| /// Creates a new Subgraph from a channel allocator and "descriptive" indices. The Subgraph's | |||
| /// index is the last element of `path`. | |||
| pub fn new_from( | |||
There was a problem hiding this comment.
I think this is better now! At least, I can't really tell why it made sense to provide the index and the path separately, and there is no self involved for which the path would make sense. I'm tempted to remove the line in the comment that you added, because "The Subgraph's index" is no longer a concept that anyone needs to know about. I would be tempted to "say more" in the comments, but the previous comments were so vague that it's hard to require it. In fact, .. I think the comments are just wrong? There is no channel allocator here yet.
What do you think about
/// Creates a `SubgraphBuilder` from a path of indexes from the dataflow root to the subgraph, terminating with the local index of the new subgraph itself.
There was a problem hiding this comment.
Ok, I totally get all the Vec<T> into Rc<[T]> stuff and think that is great. I was surprised by some other changes, but I do now think they are both correct and strict improvements in clarity, to boot! Sorry for the hold-up, and thank you for the explanations!
Signed-off-by: Moritz Hoffmann <[email protected]>
Encapsulate the path for an operator as a
Rc<[usize]>that can be sharedamong all interested parties, instead of passing owned vectors around. This
avoids some memory allocations for rendering dataflow graphs.