Add EntriesTree #149
Add EntriesTree #149
Conversation
| /// | ||
| /// Generally, this function should only be called when the `EntriesTree` | ||
| /// is first created. This will give an iterator for the children of the | ||
| /// root entry. The result of subsequent calls to this function is unspecified. |
There was a problem hiding this comment.
:-/
We should just make it take self instead of &mut self then, IMO. This would prevent subsequent calls from being possible. Any reason why you didn't pursue this?
There was a problem hiding this comment.
Or maybe assert! against calling it more than once, if we need all the EntryTreeIters to share a single &mut EntriesTree reference?
There was a problem hiding this comment.
We should just make it take self instead of &mut self then, IMO. This would prevent subsequent calls from being possible. Any reason why you didn't pursue this?
We would need to return both a new self and the iterator, and the iterator needs to be a borrow of the new self, which I don't think is possible.
Or maybe assert! against calling it more than once, if we need all the EntryTreeIters to share a single &mut EntriesTree reference?
We would need add a new flag to assert against. If we were going to take that approach, then I would prefer to instead make it possible to call iter more than once (I think this would simply need to store the starting offset of the cursor).
There was a problem hiding this comment.
I've added a commit to allow iteration more than once. Trying to get the cursor offset was complicated, so I've cloned the cursor. We can change that later if we ever add get_offset/set_offset operations on the cursor.
| /// The `EntriesTree` can be used to recursively iterate through the DIE | ||
| /// tree, following the parent/child relationships. It maintains a single | ||
| /// `EntriesCursor` that is used to parse the entries, allowing it to avoid | ||
| /// any duplicate parsing of entries. |
There was a problem hiding this comment.
It would be really nice to have an example usage here.
This allows recursive iteration of DIEs as a tree, without the caller needing to manage the depth changes, even if some levels of the tree are only partially iterated. It is implemented using a single EntriesCursor, so there is no duplicate parsing of entries, and next_sibling is used where possible. However, there is still some overhead compared to a DFS iteration: test bench_parsing_debug_info ... bench: 2,398,562 ns/iter (+/- 68,747) test bench_parsing_debug_info_tree ... bench: 2,840,048 ns/iter (+/- 72,544)
1 similar comment
This allows recursive iteration of DIEs as a tree, without the caller needing to manage the depth changes, even if some levels of the tree are only partially iterated. It is implemented using a single
EntriesCursor, so there is no duplicate parsing of entries, andnext_siblingis used where possible. However, there is still some overhead compared to a DFS iteration:See the
bench_parsing_debug_info_treebenchmark for an example of its usage.My motivation for adding this is that it allows simpler use of gimli in ruby-stacktrace. However, I'm not sure how much of that benefit will remain once gimli gains higher level interpretation of the DIE tree.