Add await-call-tree benchmark#508
Conversation
|
Can you see that this has comparable instruction count / runtime with some of the other benchmarks? In particular e.g. |
A check run goes from 150B instructions before the fix to only 233 million instructions after, while deep-vector is about 5B instructions. A debug run goes from 256B to 107B instructions, while deep-vector is only about 8B. I'm currently running a few more for comparison, not sure if this answers your question or not. |
|
It would be good to lower the nesting such that this is about the same run time as that benchmark; this is too long for our use case probably. |
The check run time is only ~15 seconds before the fix; after it's about 0.52 seconds. |
|
Removing two levels of nesting gets us to <1s on debug builds (after fix) while showing a -94% reduction in instruction count on check, so going with that. |
dc65ac3 to
1f7f47e
Compare
|
Yes, most of our benchmarks are around 1-2 seconds long. 30 seconds is getting into script-servo territory, which is the longest benchmark today I believe; we run each benchmark 3 times per "type" of which there are 3 at minimum (clean, baseline incr, clean incr, see bottom of a compare page for more details); and then for each check/opt/debug -- all in all, that's 27 times at minimum pretty much so a 30 second benchmark would add ~15 minutes of runtime which is quite a lot :) |
This sounds perfect, after fix is pretty much all that matters here I think. I'll try to get this merged tomorrow. |
Great, thanks! Once it is merged can I kick off a rust-timer run immediately, or do I need to wait for the bot to be updated? |
|
No, I'll be re-deploying the collector pretty much immediately upon merging so you'll not need to wait at all (well, give me like ~5 minutes :) |
|
Perfect. Feel free to kick off a run on rust-lang/rust#65293 if I'm not around. |
| - **await-call-tree**: A tree of 9 async fns that await each other, creating a | ||
| large type composed of many repeated `impl Future` types. Such types caused | ||
| [poor performance](https://github.com/rust-lang/rust/issues/65147) in the | ||
| past. |
There was a problem hiding this comment.
+1 for updating the docs!
There was a problem hiding this comment.
I just realized the 9 is no longer correct, removing it :)
1f7f47e to
ca5840a
Compare
To go with rust-lang/rust#65293 which fixes rust-lang/rust#65147. Locally I'm seeing a ~37x decrease in instruction count on check.
r? @Mark-Simulacrum