about summary refs log tree commit diff
diff options
context:
space:
mode:
authorManish Goregaokar <manishsmail@gmail.com>2016-10-04 15:24:03 +0530
committerGitHub <noreply@github.com>2016-10-04 15:24:03 +0530
commitad7635894ff699d2f5363eac80c70e491e3c17c4 (patch)
tree29eb4c2f4dd1df9ae1d28e91a485c70ae79926a2
parent454124974f9828a92ae26cfd8b77924301a5a510 (diff)
parent3779971dbb397ff8b1668c379812b903a6a907ec (diff)
downloadrust-ad7635894ff699d2f5363eac80c70e491e3c17c4.tar.gz
rust-ad7635894ff699d2f5363eac80c70e491e3c17c4.zip
Rollup merge of #36917 - nnethercote:speed-up-plug_leaks, r=eddyb
Speed up `plug_leaks`

Profiling shows that `plug_leaks` and the functions it calls are hot on some benchmarks. It's very common that `skol_map` is empty in this function, and we can specialize `plug_leaks` in that case for some big speed-ups.

The PR has two commits. I'm fairly confident that the first one is correct -- I traced through the code to confirm that the `fold_regions` and `pop_skolemized` calls are no-ops when `skol_map` is empty, and I also temporarily added an assertion to check that `result` ends up having the same value as `value` in that case. This commit is responsible for most of the improvement.

I'm less confident about the second commit. The call to `resolve_type_vars_is_possible` can change `value` when `skol_map` is empty... but testing suggests that it doesn't matter if the call is
omitted.

So, please check both patches carefully, especially the second one!

Here are the speed-ups for the first commit alone.

stage1 compiler (built with old rustc, using glibc malloc), doing debug builds:
```
futures-rs-test  4.710s vs  4.538s --> 1.038x faster (variance: 1.009x, 1.005x)
issue-32062-equ  0.415s vs  0.368s --> 1.129x faster (variance: 1.009x, 1.010x)
issue-32278-big  1.884s vs  1.808s --> 1.042x faster (variance: 1.020x, 1.017x)
jld-day15-parse  1.907s vs  1.668s --> 1.143x faster (variance: 1.011x, 1.007x)
piston-image-0. 13.024s vs 12.421s --> 1.049x faster (variance: 1.004x, 1.012x)
rust-encoding-0  3.335s vs  3.276s --> 1.018x faster (variance: 1.021x, 1.028x)
```
stage2 compiler (built with new rustc, using jemalloc), doing debug builds:
```
futures-rs-test  4.167s vs  4.065s --> 1.025x faster (variance: 1.006x, 1.018x)
issue-32062-equ  0.383s vs  0.343s --> 1.118x faster (variance: 1.012x, 1.016x)
issue-32278-big  1.680s vs  1.621s --> 1.036x faster (variance: 1.007x, 1.007x)
jld-day15-parse  1.671s vs  1.478s --> 1.131x faster (variance: 1.016x, 1.004x)
piston-image-0. 11.336s vs 10.852s --> 1.045x faster (variance: 1.003x, 1.006x)
rust-encoding-0  3.036s vs  2.971s --> 1.022x faster (variance: 1.030x, 1.032x)
```
I've omitted the benchmarks for which the change was negligible.

And here are the speed-ups for the first and second commit in combination.

stage1 compiler (built with old rustc, using glibc malloc), doing debug
builds:
```
futures-rs-test  4.684s vs  4.498s --> 1.041x faster (variance: 1.012x, 1.012x)
issue-32062-equ  0.413s vs  0.355s --> 1.162x faster (variance: 1.019x, 1.006x)
issue-32278-big  1.869s vs  1.763s --> 1.060x faster (variance: 1.013x, 1.018x)
jld-day15-parse  1.900s vs  1.602s --> 1.186x faster (variance: 1.010x, 1.003x)
piston-image-0. 12.907s vs 12.352s --> 1.045x faster (variance: 1.005x, 1.006x)
rust-encoding-0  3.254s vs  3.248s --> 1.002x faster (variance: 1.063x, 1.045x)
```
stage2 compiler (built with new rustc, using jemalloc), doing debug builds:
```
futures-rs-test  4.183s vs  4.046s --> 1.034x faster (variance: 1.007x, 1.004x)
issue-32062-equ  0.380s vs  0.340s --> 1.117x faster (variance: 1.020x, 1.003x)
issue-32278-big  1.671s vs  1.616s --> 1.034x faster (variance: 1.031x, 1.012x)
jld-day15-parse  1.661s vs  1.417s --> 1.172x faster (variance: 1.013x, 1.005x)
piston-image-0. 11.347s vs 10.841s --> 1.047x faster (variance: 1.007x, 1.010x)
rust-encoding-0  3.050s vs  3.000s --> 1.017x faster (variance: 1.016x, 1.012x)
```
@eddyb: `git blame` suggests that you should review this. Thanks!
-rw-r--r--src/librustc/infer/higher_ranked/mod.rs11
-rw-r--r--src/librustc/traits/project.rs2
-rw-r--r--src/librustc/traits/select.rs4
3 files changed, 9 insertions, 8 deletions
diff --git a/src/librustc/infer/higher_ranked/mod.rs b/src/librustc/infer/higher_ranked/mod.rs
index 7c02de05d26..c1d9240ba06 100644
--- a/src/librustc/infer/higher_ranked/mod.rs
+++ b/src/librustc/infer/higher_ranked/mod.rs
@@ -749,13 +749,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     pub fn plug_leaks<T>(&self,
                          skol_map: SkolemizationMap<'tcx>,
                          snapshot: &CombinedSnapshot,
-                         value: &T) -> T
+                         value: T) -> T
         where T : TypeFoldable<'tcx>
     {
         debug!("plug_leaks(skol_map={:?}, value={:?})",
                skol_map,
                value);
 
+        if skol_map.is_empty() {
+            return value;
+        }
+
         // Compute a mapping from the "taint set" of each skolemized
         // region back to the `ty::BoundRegion` that it originally
         // represented. Because `leak_check` passed, we know that
@@ -775,7 +779,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
 
         // Remove any instantiated type variables from `value`; those can hide
         // references to regions from the `fold_regions` code below.
-        let value = self.resolve_type_vars_if_possible(value);
+        let value = self.resolve_type_vars_if_possible(&value);
 
         // Map any skolemization byproducts back to a late-bound
         // region. Put that late-bound region at whatever the outermost
@@ -813,9 +817,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             }
         });
 
-        debug!("plug_leaks: result={:?}",
-               result);
-
         self.pop_skolemized(skol_map, snapshot);
 
         debug!("plug_leaks: result={:?}", result);
diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs
index ea4fc1c25ab..ddabc53a89a 100644
--- a/src/librustc/traits/project.rs
+++ b/src/librustc/traits/project.rs
@@ -171,7 +171,7 @@ pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>(
             Ok(result) => {
                 let span = obligation.cause.span;
                 match infcx.leak_check(false, span, &skol_map, snapshot) {
-                    Ok(()) => Ok(infcx.plug_leaks(skol_map, snapshot, &result)),
+                    Ok(()) => Ok(infcx.plug_leaks(skol_map, snapshot, result)),
                     Err(e) => Err(MismatchedProjectionTypes { err: e }),
                 }
             }
diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs
index 9d7131dc96c..66631111097 100644
--- a/src/librustc/traits/select.rs
+++ b/src/librustc/traits/select.rs
@@ -1980,7 +1980,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                                                   normalized_ty,
                                                   &[]);
                 obligations.push(skol_obligation);
-                this.infcx().plug_leaks(skol_map, snapshot, &obligations)
+                this.infcx().plug_leaks(skol_map, snapshot, obligations)
             })
         }).collect()
     }
@@ -2899,7 +2899,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                     predicate: predicate.value
                 }))
         }).collect();
-        self.infcx().plug_leaks(skol_map, snapshot, &predicates)
+        self.infcx().plug_leaks(skol_map, snapshot, predicates)
     }
 }