Skip to content

Speed up mbe matching in heavy recursive cases #7994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 13, 2021

Conversation

edwin0cheng
Copy link
Member

In some cases (e.g. #4186), mbe matching is very slow due to a lot of copy and allocation for bindings, this PR try to solve this problem by introduce a semi "link-list" approach for bindings building.

I used this test case (for features(32-column-tables)) to run following command to benchmark:

time rust-analyzer analysis-stats  --load-output-dirs ./ 

Before this PR : 2 mins
After this PR: 3 seconds.

However, for 64-column-tables cases, we still need 4 mins to complete.

I will try to investigate in the following weeks.

bors r+

bors bot added a commit that referenced this pull request Mar 13, 2021
7994: Speed up mbe matching in heavy recursive cases r=edwin0cheng a=edwin0cheng

In some cases (e.g.  #4186), mbe matching is very slow due to a lot of copy and allocation for bindings, this PR try to solve this problem by introduce a semi "link-list" approach for bindings building.

I used this [test case](https://github.com/weiznich/minimal_example_for_rust_81262) (for `features(32-column-tables)`) to run following command to benchmark:
```
time rust-analyzer analysis-stats  --load-output-dirs ./ 
```

Before this PR : 2 mins
After this PR: 3 seconds.

However, for 64-column-tables cases, we still need 4 mins to complete. 

I will try to investigate in the following weeks.

bors r+




Co-authored-by: Edwin Cheng <[email protected]>
@bors
Copy link
Contributor

bors bot commented Mar 13, 2021

Build failed:

@pksunkara
Copy link
Contributor

I think 4 mins for 64-column-tables is okay because it is what it was before.

@edwin0cheng
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 13, 2021

@bors bors bot merged commit fe4a94f into rust-lang:master Mar 13, 2021
@edwin0cheng edwin0cheng deleted the fix-mbe branch March 13, 2021 13:11
@edwin0cheng
Copy link
Member Author

edwin0cheng commented Mar 13, 2021

I think 4 mins for 64-column-tables is okay because it is what it was before.

IIUC, in this comment, rustc only used 26.75s in expanding. And right now we are using the same algorithm , so the performance should be similar. So there must be some bugs inside it :(

@pksunkara
Copy link
Contributor

Agree, I was just talking about RA's performance I was observing before the regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants