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

Add wg-grammar benchmark #343

Merged
merged 2 commits into from
Mar 4, 2019

Conversation

matthewjasper
Copy link
Contributor

@Mark-Simulacrum
Copy link
Member

If you could rebase that should fix CI -- well, make it run at least -- and then we can merge, I think.

@nnethercote
Copy link
Contributor

How do the instructions and max-rss numbers compare against the other benchmarks, for both non-NLL and NLL builds? It looks like you've just copied the full wg-grammar repo verbatim, and NLL builds were taking over an hour on my machine for that. We don't want this benchmark to be drastically larger than the others. (Here are instructions on how to reduce input size.)

@matthewjasper
Copy link
Contributor Author

The compile time is much better on the latest nightly. Both with and without NLL take slightly over a minute for a clean build.

@matthewjasper
Copy link
Contributor Author

I'm getting "extra arguments to rustc can only be passed to one target, consider filtering\nthe package by passing e.g. --lib or --bin NAME to specify a single target" when I try to run the benchmarks locally, do I need to remove the binary target?

@Mark-Simulacrum
Copy link
Member

Either that or add a perf-config.json file like https://github.com/rust-lang-nursery/rustc-perf/blob/master/collector/benchmarks/cargo/perf-config.json (probably without the runs: 1 bit).

@nnethercote
Copy link
Contributor

The compile time is much better on the latest nightly. Both with and without NLL take slightly over a minute for a clean build.

I'm glad NLL is much faster, but this is still much too long. Current benchmarks are all less than 30 seconds on CI, and most are less than 5 seconds. I suggest aiming for around 1-3 seconds on CI.

I care about this partly because of CI time/cost, but mostly because I often run the benchmarks locally under profilers like Cachegrind and DHAT that slow execution by up to 10x. I already have --exclude script-servo as a permanent feature of my scripts because it's too long-running (though it's currently broken). In general I'd like to avoid unnecessary increases to benchmarking times.

@Mark-Simulacrum Mark-Simulacrum merged commit 37d1300 into rust-lang:master Mar 4, 2019
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.

3 participants