Obligation forest cleanup
While looking at this code I was scratching my head about whether a node could appear in both `parent` and `dependents`. Turns out it can, but it's not useful to do so, so this PR cleans things up so it's no longer possible.
Improve memoization and refactor NLL type check
I have a big branch that is refactoring NLL type check with the goal of introducing canonicalization-based memoization for all of the operations it does. This PR contains an initial prefix of that branch which, I believe, stands alone. It does introduce a few smaller optimizations of its own:
- Skip operations that are trivially a no-op
- Cache the results of the dropck-outlives computations done by liveness
- Skip resetting unifications if nothing changed
r? @pnkfelix
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%?
```
We used to accumulate a vector of type-check constraints, but now we
generate them as we go, and just store the NLL-style
`OutlivesConstraint` and `TypeTest` information.
Avoid useless Vec clones in pending_obligations().
The only instance of `ObligationForest` in use has an obligation type of
`PendingPredicateObligation`, which contains a `PredicateObligation` and a
`Vec<Ty>`.
`FulfillmentContext::pending_obligations()` calls
`ObligationForest::pending_obligations()`, which clones all the
`PendingPredicateObligation`s. But the `Vec<Ty>` field of those cloned
obligations is never touched.
This patch changes `ObligationForest::pending_obligations()` to
`map_pending_obligations` -- which gives callers control about which part
of the obligation to clone -- and takes advantage of the change to avoid
cloning the `Vec<Ty>`. The change speeds up runs of a few rustc-perf
benchmarks, the best by 1%.
The only instance of `ObligationForest` in use has an obligation type of
`PendingPredicateObligation`, which contains a `PredicateObligation` and a
`Vec<Ty>`.
`FulfillmentContext::pending_obligations()` calls
`ObligationForest::pending_obligations()`, which clones all the
`PendingPredicateObligation`s. But the `Vec<Ty>` field of those cloned
obligations is never touched.
This patch changes `ObligationForest::pending_obligations()` to
`map_pending_obligations` -- which gives callers control about which part
of the obligation to clone -- and takes advantage of the change to avoid
cloning the `Vec<Ty>`. The change speeds up runs of a few rustc-perf
benchmarks, the best by 1%.
stabilize RangeBounds collections_range #30877
The FCP for #30877 closed last month, with the decision to:
1. move from `collections::range::RangeArgument` to `ops::RangeBounds`, and
2. rename `start()` and `end()` to `start_bounds()` and `end_bounds()`.
Simon Sapin already moved it to `ops::RangeBounds` in #49163.
I renamed the functions, and removed the old `collections::range::RangeArgument` alias.
This is my first Rust PR, please let me know if I can improve anything. This passes all tests for me, except the `clippy` tool (which uses `RangeArgument::start()`).
I considered deprecating `start()` and `end()` instead of removing them, but the contribution guidelines indicate we can break `clippy` temporarily. I thought it was best to remove the functions, since we're worried about name collisions with `Range::start` and `end`.
Closes#30877.
implement the chalk-engine traits
Preliminary implementation for the Chalk traits in rustc. Lots of `panic!()` placeholders to be filled in later.
This is currently blocked on us landing https://github.com/rust-lang-nursery/chalk/pull/131 in chalk and issuing a new release, which should occur later today.
r? @scalexm
cc @leodasvacas
stop considering location when computing outlives relationships
This doesn't (yet?) use SEME regions, but it does ignore the location for outlives constraints. This makes (I believe) NLL significantly faster -- but we should do some benchmarks. It regresses the "get-default" family of use cases for NLL, which is a shame, but keeps the other benefits, and thus represents a decent step forward.
r? @pnkfelix
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%.
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 (#2), then calls
`SnapshotMap::insert`.
- `insert` does *another* cache lookup (#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 #2. Furthermore, if the `NormalizedTy`'s
obligations are empty (the common case), we know that lookup #3 would be
a no-op, so we avoid it, while inserting a Noop into the `SnapshotMap`'s
undo log.
It was introduced in #50240 to avoid an allocation when creating a new
BTreeMap, which gave some speed-ups. But then #50352 made that the
default behaviour for BTreeMap, so LazyBTreeMap is no longer necessary.
Implement LazyBTreeMap and use it in a few places.
This is a thin wrapper around BTreeMap that avoids allocating upon creation.
I would prefer to change BTreeMap directly to make it lazy (like I did with HashSet in #36734) and I initially attempted that by making BTreeMap::root an Option<>. But then I also had to change Iter and Range to handle trees with no root, and those types have stability markers on them and I wasn't sure if that was acceptable. Also, BTreeMap has a lot of complex code and changing it all was challenging, and I didn't have high confidence about my general approach.
So I prototyped this wrapper instead and used it in the hottest locations to get some measurements about the effect. The measurements are pretty good!
- Doing a debug build of serde, it reduces the total number of heap allocations from 17,728,709 to 13,359,384, a 25% reduction. The number of bytes allocated drops from 7,474,672,966 to 5,482,308,388, a 27% reduction.
- It gives speedups of up to 3.6% on some rustc-perf benchmark jobs. crates.io, futures, and serde benefit most.
```
futures-check
avg: -1.9% min: -3.6% max: -0.5%
serde-check
avg: -2.1% min: -3.5% max: -0.7%
crates.io-check
avg: -1.7% min: -3.5% max: -0.3%
serde
avg: -2.0% min: -3.0% max: -0.9%
serde-opt
avg: -1.8% min: -2.9% max: -0.3%
futures
avg: -1.5% min: -2.8% max: -0.4%
tokio-webpush-simple-check
avg: -1.1% min: -2.2% max: -0.1%
futures-opt
avg: -1.2% min: -2.1% max: -0.4%
piston-image-check
avg: -0.8% min: -1.1% max: -0.3%
crates.io
avg: -0.6% min: -1.0% max: -0.3%
```
@Gankro, how do you think I should proceed here? Is leaving this as a wrapper reasonable? Or should I try to make BTreeMap itself lazy? If so, can I change the representation of Iter and Range?
Thanks!
Replace manual iterator exhaust with for_each(drop)
This originally added a dedicated method, `Iterator::exhaust`, and has since been replaced with `for_each(drop)`, which is more idiomatic.
<del>This is just shorthand for `for _ in &mut self {}` or `while let Some(_) = self.next() {}`. This states the intent a lot more clearly than the identical code: run the iterator to completion.
<del>At least personally, my eyes tend to gloss over `for _ in &mut self {}` without fully paying attention to what it does; having a `Drop` implementation akin to:
<del>`for _ in &mut self {}; unsafe { free(self.ptr); }`</del>
<del>Is not as clear as:
<del>`self.exhaust(); unsafe { free(self.ptr); }`
<del>Additionally, I've seen debate over whether `while let Some(_) = self.next() {}` or `for _ in &mut self {}` is more clear, whereas `self.exhaust()` is clearer than both.
