Skip to content

Conversation

@sdroege
Copy link

@sdroege sdroege commented Dec 22, 2019

@sdroege
Copy link
Author

sdroege commented Dec 22, 2019

This will probably time out on travis until the PR above is merged, it's taking quite a few minutes with latest nightly and after that PR is merged it will take about 10s.

@sdroege sdroege force-pushed the combine-issue-67454 branch from 04072bb to ef7b1b4 Compare December 22, 2019 10:18
@Mark-Simulacrum
Copy link
Member

We'll probably want to remove a few of the combine calls to get the post-PR runtime down to ~2-3 seconds at most; we try to keep the benchmarks in this repository very short as we run them lots of times while benchmarking (9 times minimum by default).

@sdroege
Copy link
Author

sdroege commented Dec 23, 2019

combine itself needs around 9s to build on my machine. The benchmark crate alone needs around 200ms with 1.40 and more than 20 minutes with current nightly.

@Mark-Simulacrum
Copy link
Member

The benchmark crate alone needs around 200ms with 1.40 and more than 20 minutes with current nightly.

Hm, so you originally said it takes around 10s after the PR -- does that mean there's still a regression vs. 1.40, or am I misinterpreting?

@sdroege
Copy link
Author

sdroege commented Dec 25, 2019

Hm, so you originally said it takes around 10s after the PR -- does that mean there's still a regression vs. 1.40, or am I misinterpreting?

Until rust-lang/rust#67454 is merged it (the benchmark crate itself, not its dependencies) will take quite a few minutes. With 1.40 and nightly after that is merged, combine itself takes about 9s on my machine and the benchmark crate itself 200ms on top of that.

@Mark-Simulacrum
Copy link
Member

Okay, so in that case, we're actually going to want to bump the runtime of the benchmark itself up a bit I guess? We want it in the 600-1500ms range I suspect.

@rylev
Copy link
Member

rylev commented Jul 9, 2021

This is pretty stale so I'm going to close for now. Feel free to reopen if you'd like to continue to work on it!

@rylev rylev closed this Jul 9, 2021
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