-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Speed up obligation forest code #51411
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
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
`process_predicates` returns a `Result<Option<Vec<PredicateObligation>>>`. `process_obligation` calls it and then fiddles with the output (using `map`, `map`, `into_iter`, `collect`) to produce a a `Result<Option<Vec<PendingPredicateObligation>>>`. This function is sufficiently hot that the fiddling is expensive. It's much better for `process_predicate` to directly return a `Result<Option<Vec<PendingPredicateObligation>>>` because `Ok(None)` accounts for ~90% of the results, and `Ok(vec![])` accounts for another ~5%.
Because it has a single callsite. This patch is large but trivial, doing the minimal amount of work (mostly `self.` insertions) necessary.
It's very hot, and this speeds things up.
A tri-valued enum is nicer than Result<Option<T>>, and it's slightly faster.
9c5dbd3
to
c83d152
Compare
@bors r+ |
📌 Commit c83d152 has been approved by |
⌛ Testing commit c83d152 with merge b541bf54dca84e576bb481a6992bf743dcebf8bd... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This is clearly an infra failure. |
@bors retry |
Speed up obligation forest code Here are the rustc-perf benchmarks that get at least a 1% speedup on one or more of their runs with these patches applied: ``` inflate-check avg: -8.7% min: -12.1% max: 0.0% inflate avg: -5.9% min: -8.6% max: 1.1% inflate-opt avg: -1.5% min: -2.0% max: -0.3% clap-rs-check avg: -0.6% min: -1.9% max: 0.5% coercions avg: -0.2%? min: -1.3%? max: 0.6%? serde-opt avg: -0.6% min: -1.0% max: 0.1% coercions-check avg: -0.4%? min: -1.0%? max: -0.0%? ```
☀️ Test successful - status-appveyor, status-travis |
Here are the rustc-perf benchmarks that get at least a 1% speedup on one or more of their runs with these patches applied: