Skip to content

Conversation

theo-lw
Copy link
Contributor

@theo-lw theo-lw commented Oct 26, 2021

Preorder iteration allocates O(size of tree) nodes. However, there are some cases where only O(height of tree) nodes are alive at a given time. I'm trying to see if using a pool allocator can help avoid excess allocations in this case.

@matklad
Copy link
Contributor

matklad commented Oct 31, 2021

Yeah, in general, it is a reasonable assumption that at any given point at time there are only few live nodes for any given tree. (although might be good to check that using rust-analyzer's analysis_stats). So it'd be cool to just add nodes to / pop them from a free-list. Ideally, tree traversal which doesn't retain nodes shouldn't do any allocation or atomic ops.

In the original implementation, this was achieved by using a thread-local free-list of nodes. As it turned out several years later, at least my implementation of that apporoach was slower than just allocating the nodes.

@theo-lw
Copy link
Contributor Author

theo-lw commented Oct 31, 2021

In the original implementation, this was achieved by using a thread-local free-list of nodes. As it turned out several years later, at least my implementation of that apporoach was slower than just allocating the nodes.

Yeah right now my implementation isn't faster than just allocating the nodes. Maybe this is a fruitless endeavour.

@theo-lw
Copy link
Contributor Author

theo-lw commented Oct 31, 2021

With my latest changes I do see a small speedup when running analysis-stats on rowan:

Without these changes:

Screen Shot 2021-10-31 at 6 45 26 PM

With these changes:

Screen Shot 2021-10-31 at 6 44 37 PM

Will probably need more benchmarks to see if this is truly significant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants