Skip to content

Commit

Permalink
Rollup merge of rust-lang#36917 - nnethercote:speed-up-plug_leaks, r=…
Browse files Browse the repository at this point in the history
…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!
  • Loading branch information
Manishearth committed Oct 4, 2016
2 parents 4541249 + 3779971 commit ad76358
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
11 changes: 6 additions & 5 deletions src/librustc/infer/higher_ranked/mod.rs
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/project.rs
Expand Up @@ -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 }),
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/select.rs
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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)
}
}

Expand Down

0 comments on commit ad76358

Please sign in to comment.