Hey, the new rowan finally at the stage where we can meaningfully compare performance and API changes.
Take a look at this PR: https://github.com/rust-analyzer/rust-analyzer/pull/7498
I'd love to see some input from @WG-rls2.0 on whether we should or should not switch to it.
The main benefit is that the API for mutating trees is much better and composable.
The main drawback is that's not zero cost -- even if you don't touch trees, there's still some fluff happening.
What about the memory usage? It looks like
mallinfo is overflowing again in your test. It goes from 392 MB to -1223 MB, which seems worse than slightly more memory.
The slowdown (5.88s to 6.33s) seems quite reasonable to me. I think assists are quite popular, and might become even more so in the future, so it's not a bad idea to invest in the API.
Agreed on both of those comment. The API improvements look really nice and should unlock some assists that would otherwise be a PITA to implement. The slowdown isn't terrible but the memory usage is concerning.
urgh, I might have left some memory leaks in?
updated the numbers: https://github.com/rust-analyzer/rust-analyzer/pull/7498#issuecomment-784359922
malloc() goes brr.. :D. That's quite an increase in memory usage
Hm, I just realized a thing which makes me more optimistic about the idea.
The fundamental trick we employ here is using different representations for “mutable” and “immutable” trees. At the moment, the two reprs are quite similar, and differ only in the number of fields.
But it feels like explicit switch between mutable and immutable tree pages the way to introducing an array-based immutable impl
That would be awesome