Skip to content

Commit 66363b2

Browse files
committed
Auto merge of #50240 - nnethercote:LazyBTreeMap, r=cramertj
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!
2 parents 68a09fc + 259ae18 commit 66363b2

File tree

5 files changed

+123
-12
lines changed

5 files changed

+123
-12
lines changed

src/librustc/infer/higher_ranked/mod.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use super::{CombinedSnapshot,
1919
use super::combine::CombineFields;
2020
use super::region_constraints::{TaintDirections};
2121

22-
use std::collections::BTreeMap;
22+
use rustc_data_structures::lazy_btree_map::LazyBTreeMap;
2323
use ty::{self, TyCtxt, Binder, TypeFoldable};
2424
use ty::error::TypeError;
2525
use ty::relate::{Relate, RelateResult, TypeRelation};
@@ -247,7 +247,8 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
247247
snapshot: &CombinedSnapshot<'a, 'tcx>,
248248
debruijn: ty::DebruijnIndex,
249249
new_vars: &[ty::RegionVid],
250-
a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
250+
a_map: &LazyBTreeMap<ty::BoundRegion,
251+
ty::Region<'tcx>>,
251252
r0: ty::Region<'tcx>)
252253
-> ty::Region<'tcx> {
253254
// Regions that pre-dated the LUB computation stay as they are.
@@ -343,7 +344,8 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
343344
snapshot: &CombinedSnapshot<'a, 'tcx>,
344345
debruijn: ty::DebruijnIndex,
345346
new_vars: &[ty::RegionVid],
346-
a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
347+
a_map: &LazyBTreeMap<ty::BoundRegion,
348+
ty::Region<'tcx>>,
347349
a_vars: &[ty::RegionVid],
348350
b_vars: &[ty::RegionVid],
349351
r0: ty::Region<'tcx>)
@@ -412,7 +414,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
412414

413415
fn rev_lookup<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
414416
span: Span,
415-
a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
417+
a_map: &LazyBTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
416418
r: ty::Region<'tcx>) -> ty::Region<'tcx>
417419
{
418420
for (a_br, a_r) in a_map {
@@ -435,7 +437,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
435437
}
436438

437439
fn var_ids<'a, 'gcx, 'tcx>(fields: &CombineFields<'a, 'gcx, 'tcx>,
438-
map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
440+
map: &LazyBTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
439441
-> Vec<ty::RegionVid> {
440442
map.iter()
441443
.map(|(_, &r)| match *r {

src/librustc/infer/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ use ty::error::{ExpectedFound, TypeError, UnconstrainedNumeric};
2828
use ty::fold::TypeFoldable;
2929
use ty::relate::RelateResult;
3030
use traits::{self, ObligationCause, PredicateObligations};
31+
use rustc_data_structures::lazy_btree_map::LazyBTreeMap;
3132
use rustc_data_structures::unify as ut;
3233
use std::cell::{Cell, RefCell, Ref, RefMut};
33-
use std::collections::BTreeMap;
3434
use std::fmt;
3535
use syntax::ast;
3636
use errors::DiagnosticBuilder;
@@ -187,7 +187,7 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
187187

188188
/// A map returned by `skolemize_late_bound_regions()` indicating the skolemized
189189
/// region that each late-bound region was replaced with.
190-
pub type SkolemizationMap<'tcx> = BTreeMap<ty::BoundRegion, ty::Region<'tcx>>;
190+
pub type SkolemizationMap<'tcx> = LazyBTreeMap<ty::BoundRegion, ty::Region<'tcx>>;
191191

192192
/// See `error_reporting` module for more details
193193
#[derive(Clone, Debug)]
@@ -1216,7 +1216,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
12161216
span: Span,
12171217
lbrct: LateBoundRegionConversionTime,
12181218
value: &ty::Binder<T>)
1219-
-> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
1219+
-> (T, LazyBTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
12201220
where T : TypeFoldable<'tcx>
12211221
{
12221222
self.tcx.replace_late_bound_regions(

src/librustc/ty/fold.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ use middle::const_val::ConstVal;
4343
use hir::def_id::DefId;
4444
use ty::{self, Binder, Ty, TyCtxt, TypeFlags};
4545

46+
use rustc_data_structures::lazy_btree_map::LazyBTreeMap;
4647
use std::fmt;
47-
use std::collections::BTreeMap;
4848
use util::nodemap::FxHashSet;
4949

5050
/// The TypeFoldable trait is implemented for every type that can be folded.
@@ -334,7 +334,7 @@ struct RegionReplacer<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
334334
tcx: TyCtxt<'a, 'gcx, 'tcx>,
335335
current_depth: u32,
336336
fld_r: &'a mut (dyn FnMut(ty::BoundRegion) -> ty::Region<'tcx> + 'a),
337-
map: BTreeMap<ty::BoundRegion, ty::Region<'tcx>>
337+
map: LazyBTreeMap<ty::BoundRegion, ty::Region<'tcx>>
338338
}
339339

340340
impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
@@ -349,7 +349,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
349349
pub fn replace_late_bound_regions<T,F>(self,
350350
value: &Binder<T>,
351351
mut f: F)
352-
-> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
352+
-> (T, LazyBTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
353353
where F : FnMut(ty::BoundRegion) -> ty::Region<'tcx>,
354354
T : TypeFoldable<'tcx>,
355355
{
@@ -462,7 +462,7 @@ impl<'a, 'gcx, 'tcx> RegionReplacer<'a, 'gcx, 'tcx> {
462462
tcx,
463463
current_depth: 1,
464464
fld_r,
465-
map: BTreeMap::default()
465+
map: LazyBTreeMap::default()
466466
}
467467
}
468468
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use std::collections::btree_map;
12+
use std::collections::BTreeMap;
13+
14+
/// A thin wrapper around BTreeMap that avoids allocating upon creation.
15+
///
16+
/// Vec, HashSet and HashMap all have the nice feature that they don't do any
17+
/// heap allocation when creating a new structure of the default size. In
18+
/// contrast, BTreeMap *does* allocate in that situation. The compiler uses
19+
/// B-Tree maps in some places such that many maps are created but few are
20+
/// inserted into, so having a BTreeMap alternative that avoids allocating on
21+
/// creation is a performance win.
22+
///
23+
/// Only a fraction of BTreeMap's functionality is currently supported.
24+
/// Additional functionality should be added on demand.
25+
#[derive(Debug)]
26+
pub struct LazyBTreeMap<K, V>(Option<BTreeMap<K, V>>);
27+
28+
impl<K, V> LazyBTreeMap<K, V> {
29+
pub fn new() -> LazyBTreeMap<K, V> {
30+
LazyBTreeMap(None)
31+
}
32+
33+
pub fn iter(&self) -> Iter<K, V> {
34+
Iter(self.0.as_ref().map(|btm| btm.iter()))
35+
}
36+
37+
pub fn is_empty(&self) -> bool {
38+
self.0.as_ref().map_or(true, |btm| btm.is_empty())
39+
}
40+
}
41+
42+
impl<K: Ord, V> LazyBTreeMap<K, V> {
43+
fn instantiate(&mut self) -> &mut BTreeMap<K, V> {
44+
if let Some(ref mut btm) = self.0 {
45+
btm
46+
} else {
47+
let btm = BTreeMap::new();
48+
self.0 = Some(btm);
49+
self.0.as_mut().unwrap()
50+
}
51+
}
52+
53+
pub fn insert(&mut self, key: K, value: V) -> Option<V> {
54+
self.instantiate().insert(key, value)
55+
}
56+
57+
pub fn entry(&mut self, key: K) -> btree_map::Entry<K, V> {
58+
self.instantiate().entry(key)
59+
}
60+
61+
pub fn values<'a>(&'a self) -> Values<'a, K, V> {
62+
Values(self.0.as_ref().map(|btm| btm.values()))
63+
}
64+
}
65+
66+
impl<K: Ord, V> Default for LazyBTreeMap<K, V> {
67+
fn default() -> LazyBTreeMap<K, V> {
68+
LazyBTreeMap::new()
69+
}
70+
}
71+
72+
impl<'a, K: 'a, V: 'a> IntoIterator for &'a LazyBTreeMap<K, V> {
73+
type Item = (&'a K, &'a V);
74+
type IntoIter = Iter<'a, K, V>;
75+
76+
fn into_iter(self) -> Iter<'a, K, V> {
77+
self.iter()
78+
}
79+
}
80+
81+
pub struct Iter<'a, K: 'a, V: 'a>(Option<btree_map::Iter<'a, K, V>>);
82+
83+
impl<'a, K: 'a, V: 'a> Iterator for Iter<'a, K, V> {
84+
type Item = (&'a K, &'a V);
85+
86+
fn next(&mut self) -> Option<(&'a K, &'a V)> {
87+
self.0.as_mut().and_then(|iter| iter.next())
88+
}
89+
90+
fn size_hint(&self) -> (usize, Option<usize>) {
91+
self.0.as_ref().map_or_else(|| (0, Some(0)), |iter| iter.size_hint())
92+
}
93+
}
94+
95+
pub struct Values<'a, K: 'a, V: 'a>(Option<btree_map::Values<'a, K, V>>);
96+
97+
impl<'a, K, V> Iterator for Values<'a, K, V> {
98+
type Item = &'a V;
99+
100+
fn next(&mut self) -> Option<&'a V> {
101+
self.0.as_mut().and_then(|values| values.next())
102+
}
103+
104+
fn size_hint(&self) -> (usize, Option<usize>) {
105+
self.0.as_ref().map_or_else(|| (0, Some(0)), |values| values.size_hint())
106+
}
107+
}
108+

src/librustc_data_structures/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ pub mod bitvec;
6060
pub mod graph;
6161
pub mod indexed_set;
6262
pub mod indexed_vec;
63+
pub mod lazy_btree_map;
6364
pub mod obligation_forest;
6465
pub mod sip128;
6566
pub mod snapshot_map;

0 commit comments

Comments
 (0)