Skip to content
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

poc: remove Rc Cell from parser2 parse scope structs #874

Draft
wants to merge 69 commits into
base: fe-v2
Choose a base branch
from

Conversation

sbillig
Copy link
Collaborator

@sbillig sbillig commented Apr 19, 2023

I've been wasting my time profiling the new parser. I noticed that it does a decent amount of allocating, and figured there may be easy speed wins. I'm fully aware that no one cares about this and I have better things to do with my life.

Here's an ugly poc of a minor improvement from a simple change; putting it here so that it doesn't fade away in my pile of git stashes.

Parsing a slightly modified uniswap contract:
739.23 µs -> 647.37 µs

(This pr is based on #860)

@sbillig sbillig changed the base branch from master to fe-v2 April 19, 2023 22:15
@Y-Nak
Copy link
Member

Y-Nak commented Apr 19, 2023

Nice! This is Interesting.
I tried to run rust-analyzer's benchmark for its parser.
It takes around 20-40ms for 8000 lines file(you may need to modify the test code slightly, it might have a bug).
The command was

# in rust-analyzer/crates/syntax
RUN_SLOW_BENCHES=1 cargo test --release -- --nocapture --test-threads=1

Also, I tried running the benchmark on 9,000 lines file made by repeating uniswap-paraser2.fe about 20 times. It seemed to take about 20ms to execute.

So, it depends on the later passes, but I guess it should be fine for now.

@sbillig
Copy link
Collaborator Author

sbillig commented Apr 19, 2023

I agree, but if we remove the Rc Cell, it could take about 17ms! ;)

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