Stream: t-compiler/rust-analyzer

Topic: Should we switch to the new rowan?


matklad (Feb 23 2021 at 17:13, on Zulip):

Hey, the new rowan finally at the stage where we can meaningfully compare performance and API changes.

matklad (Feb 23 2021 at 17:14, on Zulip):

Take a look at this PR: https://github.com/rust-analyzer/rust-analyzer/pull/7498

matklad (Feb 23 2021 at 17:15, on Zulip):

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.

Laurențiu (Feb 23 2021 at 17:25, on Zulip):

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.

Laurențiu (Feb 23 2021 at 17:28, on Zulip):

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.

Jeremy Kolb (Feb 23 2021 at 17:31, on Zulip):

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.

matklad (Feb 23 2021 at 18:01, on Zulip):

urgh, I might have left some memory leaks in?

matklad (Feb 23 2021 at 18:49, on Zulip):

updated the numbers: https://github.com/rust-analyzer/rust-analyzer/pull/7498#issuecomment-784359922

Laurențiu (Feb 23 2021 at 18:51, on Zulip):

malloc() goes brr.. :D. That's quite an increase in memory usage

matklad (Feb 23 2021 at 19:02, on Zulip):

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

Laurențiu (Feb 23 2021 at 19:14, on Zulip):

That would be awesome

Last update: Jul 28 2021 at 03:30UTC