Speed up opt_normalize_projection_type#50818
Merged
bors merged 2 commits intorust-lang:masterfrom May 18, 2018
Merged
Conversation
There is a hot path through `opt_normalize_projection_type`: - `try_start` does a cache lookup (#1). - The result is a `NormalizedTy`. - There are no unresolved type vars, so we call `complete`. - `complete` does *another* cache lookup (rust-lang#2), then calls `SnapshotMap::insert`. - `insert` does *another* cache lookup (rust-lang#3), inserting the same value that's already in the cache. This patch optimizes this hot path by introducing `complete_normalized`, for use when the value is known in advance to be a `NormalizedTy`. It always avoids lookup rust-lang#2. Furthermore, if the `NormalizedTy`'s obligations are empty (the common case), we know that lookup rust-lang#3 would be a no-op, so we avoid it, while inserting a Noop into the `SnapshotMap`'s undo log.
This patch changes `opt_normalize_project_type` so it appends obligations to a given obligations vector, instead of returning a new obligations vector. This change avoids lots of allocations. In the most extreme case, for a clean "Check" build of serde it reduces the total number of allocations by 20%.
Contributor
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| infer::LateBoundRegionConversionTime::HigherRankedType, | ||
| data); | ||
| let normalized = super::normalize_projection_type( | ||
| let mut obligations = vec![]; |
Member
There was a problem hiding this comment.
If this specifically is hot, it'd be interesting to see if we could make this an Option argument since obligations appear to be unused in this case.
Contributor
Author
There was a problem hiding this comment.
This particular callsite is not hot. The hot callsite is the one within <TypeFolder for AssociatedTypeNormalizer>::fold_ty in librustc/traits/project.rs, which passes an existing vector.
Member
nikomatsakis
approved these changes
May 17, 2018
Contributor
|
@bors r+ |
Collaborator
|
📌 Commit 47bc774 has been approved by |
bors
added
S-waiting-on-bors
Member
|
@bors rollup |
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
May 17, 2018
…omatsakis Speed up `opt_normalize_projection_type` `opt_normalize_projection_type` is hot in the serde and futures benchmarks in rustc-perf. These two patches speed up the execution of most runs for them by 2--4%.
bors
added a commit
that referenced
this pull request
May 18, 2018
Rollup of 10 pull requests Successful merges: - #50387 (Remove leftover tab in libtest outputs) - #50553 (Add Option::xor method) - #50610 (Improve format string errors) - #50649 (Tweak `nearest_common_ancestor()`.) - #50790 (Fix grammar documentation wrt Unicode identifiers) - #50791 (Fix null exclusions in grammar docs) - #50806 (Add `bless` x.py subcommand for easy ui test replacement) - #50818 (Speed up `opt_normalize_projection_type`) - #50837 (Revert #49767) - #50839 (Make sure people know the book is free oline) Failed merges:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
opt_normalize_projection_typeis hot in the serde and futures benchmarks in rustc-perf. These two patches speed up the execution of most runs for them by 2--4%.