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

Optimize strnom::whitespace #126

Closed
dtolnay opened this issue Sep 3, 2018 · 2 comments · May be fixed by #202
Closed

Optimize strnom::whitespace #126

dtolnay opened this issue Sep 3, 2018 · 2 comments · May be fixed by #202

Comments

@dtolnay
Copy link
Owner

dtolnay commented Sep 3, 2018

I wrote a benchmark that uses Syn to parse everything in the rust-lang/rust repo + submodules and it spends more than 10% of its time in this function. It would be good to optimize as much as possible if anyone is interested -- but without increasing compile time of proc-macro2.

Here is a minimal benchmark to reproduce and optimize:

// [dependencies]
// proc-macro2 = { version = "0.4", default-features = false }

fn main() {
    let path = "../rust/src/libsyntax/parse/parser.rs";
    let content = std::fs::read_to_string(path).unwrap();
    for _ in 0..1000 {
        content.parse::<proc_macro2::TokenStream>().unwrap();
    }
}
$ cargo build --release
$ valgrind --tool=callgrind target/release/bench-procmacro2
$ callgrind_annotate --auto=yes callgrind.out.*

In this program 41% of instructions executed are associated with strnom::whitespace.

@mystor
Copy link
Contributor

mystor commented Sep 4, 2018

I'm guessing the cause of this problem is due to whitespace being called repeatedly when attempting different token types. Currently we skip whitespace in the start of each parser primitive, rather than having a single point where we parse whitespace. Namely punct!() parses whitespace for each time it is called, when we should usually just advance whitespace once.

The stable parser in proc_macro2 could probably use a rewrite in the style of syn's recent rewrite to be more procedural, optimized, and streamlined.

@dtolnay
Copy link
Owner Author

dtolnay commented May 22, 2020

I think this is done. As of 1.0.14 we spend <5% of time in skip_whitespace and take_until_newline_or_eof totalled together.

@dtolnay dtolnay closed this as completed May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants