rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.05k stars 1.56k forks source link

MBE are rather slow #5549

Open lnicola opened 4 years ago

lnicola commented 4 years ago

Making https://github.com/rust-analyzer/rust-analyzer/blob/19f891250647b9c08cf9d5f23433ec7380a85010/crates/ra_mbe/src/mbe_expander.rs#L17 return Some(ExpandError::NoMatchingRule) seems to speed up parsing by some 45%, as measured by rust-analyzer analysis-stats .. There might be some potential for optimization.

CC #5426.

edwin0cheng commented 3 years ago

I did some profiling on the whole rust-analyzer process :

cargo flamegraph --bin=rust-analyzer -- -q analysis-stats .

https://raw.githubusercontent.com/gist/edwin0cheng/c041f4597896e749b2788d34f0074de8/raw/c7fc242bd689ce03a22e32ad80ae9b8a12fb4447/flamegraph.svg

The actual mbe related code is around 0.4% of time, such that is quite hard to improve the performance it solely. (I did make some improvement and refactor, but it can't make it noticeable)

lnicola commented 3 years ago

How do you search in that flamegraph? This thing here does nothing for me: EDIT: I have to download the file.

image

I think perf puts mbe::syntax_bridge::token_tree_to_syntax_node at 9.2% (but I've always been confused by perf report):

image

but that comes from parser::parse_fragment, which is arguably not MBE.

Regarding the 45% difference that I've got, there might be another explanation for it: without MBE, there's less code to parse though I'm not sure how much is MBE and how much is the parser.

lnicola commented 3 years ago

Here's the one I get (with debug info bumped up): https://gist.github.com/lnicola/8dfbe8c5c1bb2b2efe2d8adb9463fbd9#file-flamegraph-svg.

edwin0cheng commented 3 years ago

I think perf puts mbe::syntax_bridge::token_tree_to_syntax_node at 9.2% (but I've always been confused by perf report)

Oh, seem like I missed that, I will try to improve it a bit. But I agree on you explanation about the mbe expansion parsing. Even a simple macro expansion could lead to parsing more code exponentially. Anyway, I will do more investigation :)

edwin0cheng commented 3 years ago

I just collected some stats for mbe, which is for finding out the actual number of how many things need to expanded (collected by adding some println in https://github.com/rust-analyzer/rust-analyzer/blob/ed732e86eb88393cdec471b263303adea6ffcb73/crates/hir_expand/src/db.rs#L237 and then running `rust-analyzer analysis-stat .' :

MacroFile duplicates
#number of duplicates : #count
---------------------------------
0 : 42382
1 : 1882
2 : 861
3 : 29
4 : 1
---------------------------------
number of non duplicated MacroFile = 45155
total number of MacroFile = 48850

The figure above indicate how well salsa cached our result which is quite good, only 8% of MacroFile needs to recompute (but maybe it is because the nature of analysis-stat ?)

input duplicates (Removed duplicated MacroFile)
---------------------------------
#number of duplicates : #count
0 : 27629
1 : 1390
2 : 313
3 : 105
4 : 51
5 : 42
6 : 20
7 : 18
8 : 10
9 : 11
10-49 : 161
50-99 : 14
100-199 : 8
200-499 : 11
500-699 : 0
700-999 : 3
---------------------------------

Figures above is quite interesting, which is counting the duplicates of input (the content of macro_rules and macro_arg). So if it is corrected, there are almost a half of macro inputs are duplicated. In fact, I can imagine some recursive macro will generate a lot of duplicated things and that's why the numbers shown in here.

So next step maybe try to cache the input (e.g. compuate a sha hash) and reuse it ?