introduce canonical queries, use for normalization and dropck-outlives#48411

nikomatsakis · 2018-02-21T22:21:12Z

This branch adds in the concept of a canonicalized trait query and uses it for three specific operations:

infcx.at(cause, param_env).normalize(type_foldable)
- normalizes all associated types in type_foldable
tcx.normalize_erasing_regions(param_env, type_foldable)
- like normalize, but erases regions first and in the result; this leads to better caching
infcx.at(cause, param_env).dropck_outlives(ty)
- produces the set of types that must be live when a value of type ty is dropped
- used from dropck but also NLL outlives

This is a kind of "first step" towards a more Chalk-ified approach. It leads to a big speedup for NLL, which is basically dominated by the dropck-outlives computation. Here are some timing measurements for the syn crate (pre-branch measurements coming soon):

Commit	NLL disabled	NLL enabled
Before my branch	5.43s	8.99s
After my branch	5.36s	7.25s

(Note that NLL enabled still does all the work that NLL disabled does, so this is not really a way to compare the performance of NLL versus the AST-based borrow checker directly.) Since this affects all codepaths, I'd like to do a full perf run before we land anything.

Also, this is not the "final point" for canonicalization etc. I think canonicalization can be made substantially faster, for one thing. But it seems like a reasonable starting point for a branch that's gotten a bit larger than I would have liked.

Commit convention: First of all, this entire branch ought to be a "pure refactoring", I believe, not changing anything about external behavior. Second, I've tagged the most important commits with [VIC] (very important commit), so you can scan for those. =)

r? @eddyb

eddyb · 2018-02-21T23:16:09Z

src/librustc_traits/normalize_erasing_regions.rs

+        let cause = ObligationCause::dummy();
+        match infcx.at(&cause, param_env).normalize(&value) {
+            Ok(Normalized { value: normalized_value, obligations: _ }) => {
+                //                                   ^^^^^^^^^^^


How certain is this? Can you assert that this holds, or would that be too expensive?

Hmm. It's probably worth asserting, or maybe refactoring the types to prove it.

eddyb · 2018-02-21T23:20:03Z

src/librustc_trans/mir/mod.rs

-        where T: TransNormalize<'tcx>
+        where T: TypeFoldable<'tcx>
    {
        self.cx.tcx.trans_apply_param_substs(self.param_substs, value)


Can we get rid of trans_apply_param_substs now?

eddyb · 2018-02-21T23:21:16Z

r=me with nits and build failure addressed

nikomatsakis · 2018-02-22T10:14:24Z

@bors try

I'd like to do a perf run.

bors · 2018-02-22T10:14:37Z

⌛ Trying commit 6d787d4 with merge fc2b29d...

@eddyb

…try> introduce canonical queries, use for normalization and dropck-outlives This branch adds in the concept of a **canonicalized trait query** and uses it for three specific operations: - `infcx.at(cause, param_env).normalize(type_foldable)` - normalizes all associated types in `type_foldable` - `tcx.normalize_erasing_regions(param_env, type_foldable)` - like normalize, but erases regions first and in the result; this leads to better caching - `infcx.at(cause, param_env).dropck_outlives(ty)` - produces the set of types that must be live when a value of type `ty` is dropped - used from dropck but also NLL outlives This is a kind of "first step" towards a more Chalk-ified approach. It leads to a **big** speedup for NLL, which is basically dominated by the dropck-outlives computation. Here are some timing measurements for the `syn` crate (pre-branch measurements coming soon): | Commit | NLL disabled | NLL enabled | | ------- | --- | --- | | Before my branch | 5.43s | 8.99s | | After my branch | 5.36s | 7.25s | (Note that NLL enabled still does *all the work* that NLL disabled does, so this is not really a way to compare the performance of NLL versus the AST-based borrow checker directly.) Since this affects all codepaths, I'd like to do a full perf run before we land anything. Also, this is not the "final point" for canonicalization etc. I think canonicalization can be made substantially faster, for one thing. But it seems like a reasonable starting point for a branch that's gotten a bit larger than I would have liked. **Commit convention:** First of all, this entire branch ought to be a "pure refactoring", I believe, not changing anything about external behavior. Second, I've tagged the most important commits with `[VIC]` (very important commit), so you can scan for those. =) r? @eddyb

bors · 2018-02-22T12:22:02Z

☀️ Test successful - status-travis
State: approved= try=True

nikomatsakis · 2018-02-22T14:17:37Z

cc @rust-lang/release -- I'd like to request a perf run. try build completed, not sure what other steps I need to do.

Mark-Simulacrum · 2018-02-22T17:38:58Z

I'll run it soon, for now just always ping me

Mark-Simulacrum · 2018-02-22T18:39:14Z

Perf started

nikomatsakis · 2018-02-22T22:43:20Z

Here are the results:

perf comparison

Also archived here, since I've noticed that perf links tend to go stale:

http://smallcultfollowing.com/pr-48411/rustc%20performance%20data.htm (instructions)
http://smallcultfollowing.com/pr-48411/times.html (times)

nikomatsakis · 2018-02-22T22:54:20Z

Major changes (>10%)

Got worse:

futures/futures-opt: +10-15%
regex: 34.4% (patched incremental: compile one)
regex-opt: 56% (patched incremental: compile one-opt)

Got better

crates.io-opt: -30.0% on println-opt
regression-31157, regression-31157-opt: -10% or so
style-servo-opt: -21.0% (clean-incremental-opt)

Medium changes (3-10%)

coercions/coercions-opt: +4%
piston-image: +6% (clean-incremental, clean-incremental-opt)
unify-linearly: +9% (clean)

Got better:

tokio-webpush-simple: -5%
tokio-webpush-simple-opt: -2% through -5%
unify-linearly: -4% (dummy fn)

Small changes (<3%)

syn: -2%
crates.io: -2.5% mostly (one change +1%)
deep-vector: +1%
htm5ever: +2%
html5ever-opt: +1%
hyper: +1%, -2%
parser, parser-opt: +2%
piston-image: -1%, +6%
tuple-stress: +2%

nikomatsakis · 2018-02-22T22:54:37Z

I'll try to take a look at regex and regex-opt.

nikomatsakis · 2018-02-23T21:44:51Z

@bors try

bors · 2018-02-23T21:44:56Z

🔒 Merge conflict

nikomatsakis · 2018-02-24T00:02:22Z

@bors try

@eddyb

…try> introduce canonical queries, use for normalization and dropck-outlives This branch adds in the concept of a **canonicalized trait query** and uses it for three specific operations: - `infcx.at(cause, param_env).normalize(type_foldable)` - normalizes all associated types in `type_foldable` - `tcx.normalize_erasing_regions(param_env, type_foldable)` - like normalize, but erases regions first and in the result; this leads to better caching - `infcx.at(cause, param_env).dropck_outlives(ty)` - produces the set of types that must be live when a value of type `ty` is dropped - used from dropck but also NLL outlives This is a kind of "first step" towards a more Chalk-ified approach. It leads to a **big** speedup for NLL, which is basically dominated by the dropck-outlives computation. Here are some timing measurements for the `syn` crate (pre-branch measurements coming soon): | Commit | NLL disabled | NLL enabled | | ------- | --- | --- | | Before my branch | 5.43s | 8.99s | | After my branch | 5.36s | 7.25s | (Note that NLL enabled still does *all the work* that NLL disabled does, so this is not really a way to compare the performance of NLL versus the AST-based borrow checker directly.) Since this affects all codepaths, I'd like to do a full perf run before we land anything. Also, this is not the "final point" for canonicalization etc. I think canonicalization can be made substantially faster, for one thing. But it seems like a reasonable starting point for a branch that's gotten a bit larger than I would have liked. **Commit convention:** First of all, this entire branch ought to be a "pure refactoring", I believe, not changing anything about external behavior. Second, I've tagged the most important commits with `[VIC]` (very important commit), so you can scan for those. =) r? @eddyb

bors · 2018-02-24T00:02:53Z

⌛ Trying commit 41d762f with merge 251f865...

bors · 2018-02-24T02:43:01Z

☀️ Test successful - status-travis
State: approved= try=True

Mark-Simulacrum · 2018-02-24T03:14:52Z

Perf started for 251f865.

nikomatsakis · 2018-02-24T12:02:21Z

@Mark-Simulacrum when I try to compare,I get errors about not having results for 063deba ... am I doing something wrong ? =)

Consolidate `trans_apply_param_substs` and `trans_apply_param_substs_env`. Also remove `trans_impl_self_ty`

nikomatsakis · 2018-03-13T15:22:28Z

@bors r=eddyb

bors · 2018-03-13T15:22:29Z

📌 Commit 17c4103 has been approved by eddyb

bors · 2018-03-13T15:24:52Z

⌛ Testing commit 17c4103 with merge 8c4ff22...

@eddyb

…ddyb introduce canonical queries, use for normalization and dropck-outlives This branch adds in the concept of a **canonicalized trait query** and uses it for three specific operations: - `infcx.at(cause, param_env).normalize(type_foldable)` - normalizes all associated types in `type_foldable` - `tcx.normalize_erasing_regions(param_env, type_foldable)` - like normalize, but erases regions first and in the result; this leads to better caching - `infcx.at(cause, param_env).dropck_outlives(ty)` - produces the set of types that must be live when a value of type `ty` is dropped - used from dropck but also NLL outlives This is a kind of "first step" towards a more Chalk-ified approach. It leads to a **big** speedup for NLL, which is basically dominated by the dropck-outlives computation. Here are some timing measurements for the `syn` crate (pre-branch measurements coming soon): | Commit | NLL disabled | NLL enabled | | ------- | --- | --- | | Before my branch | 5.43s | 8.99s | | After my branch | 5.36s | 7.25s | (Note that NLL enabled still does *all the work* that NLL disabled does, so this is not really a way to compare the performance of NLL versus the AST-based borrow checker directly.) Since this affects all codepaths, I'd like to do a full perf run before we land anything. Also, this is not the "final point" for canonicalization etc. I think canonicalization can be made substantially faster, for one thing. But it seems like a reasonable starting point for a branch that's gotten a bit larger than I would have liked. **Commit convention:** First of all, this entire branch ought to be a "pure refactoring", I believe, not changing anything about external behavior. Second, I've tagged the most important commits with `[VIC]` (very important commit), so you can scan for those. =) r? @eddyb

bors · 2018-03-13T18:21:03Z

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 8c4ff22 to master...

@nikomatsakis

[WIP] Create a canonical trait query for `evaluate_obligation` This builds on the canonical query machinery introduced in #48411 to introduce a new canonical trait query for `evaluate_obligation` in the trait selector. Also ports most callers of the original `evaluate_obligation` to the new system (except in coherence, which requires support for intercrate mode). Closes #48537. r? @nikomatsakis

@nikomatsakis

[WIP] Create a canonical trait query for `evaluate_obligation` This builds on the canonical query machinery introduced in #48411 to introduce a new canonical trait query for `evaluate_obligation` in the trait selector. Also ports most callers of the original `evaluate_obligation` to the new system (except in coherence, which requires support for intercrate mode). Closes #48537. r? @nikomatsakis

@nikomatsakis

Create a canonical trait query for `evaluate_obligation` This builds on the canonical query machinery introduced in #48411 to introduce a new canonical trait query for `evaluate_obligation` in the trait selector. Also ports most callers of the original `evaluate_obligation` to the new system (except in coherence, which requires support for intercrate mode). Closes #48537. r? @nikomatsakis

RalfJung · 2018-07-29T01:22:23Z

FWIW, this (specifically, short-circuit dropck_outlives for simple cases ) caused a soundness bug: #52786

rust-highfive assigned eddyb Feb 21, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 21, 2018

eddyb reviewed Feb 21, 2018

View reviewed changes

nikomatsakis mentioned this pull request Feb 22, 2018

🛠 improve canonicalization by using a hashing scheme #48417

Closed

nikomatsakis mentioned this pull request Feb 22, 2018

NLL performance tracking issue #47879

Closed

6 tasks

emilyalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2018

nikomatsakis force-pushed the chalkify-canonical-query-mir branch 2 times, most recently from 0e84c2d to 41d762f Compare February 23, 2018 22:44

eternaleye mentioned this pull request Feb 24, 2018

Parametrise modules rust-lang/rfcs#424

Open

nikomatsakis added 5 commits March 13, 2018 11:22

trans_apply_param_substs => subst_and_normalize_erasing_regions

6288faa

Consolidate `trans_apply_param_substs` and `trans_apply_param_substs_env`. Also remove `trans_impl_self_ty`

add a debug assertion that only outlives-oblig. result from norm.

fc04c41

replace inline docs with references to rustc-guide

d326738

remove dead code

29dc902

add "text" sections for things that seem likely to be a problem

17c4103

nikomatsakis force-pushed the chalkify-canonical-query-mir branch from fbaf9cb to 17c4103 Compare March 13, 2018 15:22

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 13, 2018

alexcrichton mentioned this pull request Mar 13, 2018

Update sccache to its master branch #48647

Merged

nikomatsakis mentioned this pull request Mar 13, 2018

NLL should identify and respect the lifetime annotations that the user wrote #48482

Merged

bors merged commit 17c4103 into rust-lang:master Mar 13, 2018

aravind-pg mentioned this pull request Mar 13, 2018

Create a canonical trait query for evaluate_obligation #48995

Merged

nikomatsakis mentioned this pull request Mar 13, 2018

NLL should identify and respect the lifetime annotations that the user wrote #47184

Closed

27 tasks

davidtwco mentioned this pull request Apr 3, 2018

NLL breaks functions with intermingled lifetimes in return type when returned value is a (non-borrowed) associated type #49354

Closed

emilyalbini mentioned this pull request Apr 17, 2018

[ICE] librustc_traits/normalize_erasing_regions.rs:46: could not fully normalize ... #49685

Closed

Uh oh!

Conversation

nikomatsakis commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eddyb Feb 21, 2018

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Feb 21, 2018

Choose a reason for hiding this comment

Uh oh!

eddyb Feb 21, 2018

Choose a reason for hiding this comment

Uh oh!

eddyb commented Feb 21, 2018

Uh oh!

nikomatsakis commented Feb 22, 2018

Uh oh!

bors commented Feb 22, 2018

Uh oh!

bors commented Feb 22, 2018

Uh oh!

nikomatsakis commented Feb 22, 2018

Uh oh!

Mark-Simulacrum commented Feb 22, 2018

Uh oh!

Mark-Simulacrum commented Feb 22, 2018

Uh oh!

nikomatsakis commented Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikomatsakis commented Feb 22, 2018

Major changes (>10%)

Medium changes (3-10%)

Small changes (<3%)

Uh oh!

nikomatsakis commented Feb 22, 2018

Uh oh!

nikomatsakis commented Feb 23, 2018

Uh oh!

bors commented Feb 23, 2018

Uh oh!

nikomatsakis commented Feb 24, 2018

Uh oh!

bors commented Feb 24, 2018

Uh oh!

bors commented Feb 24, 2018

Uh oh!

Mark-Simulacrum commented Feb 24, 2018

Uh oh!

nikomatsakis commented Feb 24, 2018

Uh oh!

nikomatsakis commented Mar 13, 2018

Uh oh!

bors commented Mar 13, 2018

Uh oh!

bors commented Mar 13, 2018

Uh oh!

bors commented Mar 13, 2018

Uh oh!

RalfJung commented Jul 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

nikomatsakis commented Feb 21, 2018 •

edited

Loading

nikomatsakis commented Feb 22, 2018 •

edited

Loading

RalfJung commented Jul 29, 2018 •

edited

Loading