From 4459fe3e5ef84bc25a93c7c388006465b157dab2 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 4 Jul 2014 18:47:59 +0200 Subject: [PATCH] Revise `VecPerParamSpace` to use a one Vec rather than three. In my informal measurements, this brings the peak memory usage when building librustc from 1662M down to 1502M. Since 1662 - 1502 = 160, this may not recover the entirety of the observed memory regression (250M) from PR #14604. (However, according to my local measurements, the regression when building librustc was more like 209M, so perhaps this will still recover the lions share of the lost memory.) --- src/librustc/middle/subst.rs | 157 ++++++++++++++++++++++++++--------- 1 file changed, 116 insertions(+), 41 deletions(-) diff --git a/src/librustc/middle/subst.rs b/src/librustc/middle/subst.rs index ec2321d5d24..4684bd3532e 100644 --- a/src/librustc/middle/subst.rs +++ b/src/librustc/middle/subst.rs @@ -15,7 +15,6 @@ use middle::ty_fold; use middle::ty_fold::{TypeFoldable, TypeFolder}; use util::ppaux::Repr; -use std::iter::Chain; use std::mem; use std::raw; use std::slice::{Items, MutItems}; @@ -191,8 +190,8 @@ impl Substs { } pub fn with_method_from(self, substs: &Substs) -> Substs { - self.with_method((*substs.types.get_vec(FnSpace)).clone(), - (*substs.regions().get_vec(FnSpace)).clone()) + self.with_method(Vec::from_slice(substs.types.get_slice(FnSpace)), + Vec::from_slice(substs.regions().get_slice(FnSpace))) } pub fn with_method(self, @@ -261,19 +260,44 @@ impl ParamSpace { */ #[deriving(PartialEq, Eq, Clone, Hash, Encodable, Decodable)] pub struct VecPerParamSpace { - vecs: (Vec, Vec, Vec) + // This was originally represented as a tuple with one Vec for + // each variant of ParamSpace, and that remains the abstraction + // that it provides to its clients. + // + // Here is how the representation corresponds to the abstraction + // i.e. the "abstraction function" AF: + // + // AF(self) = (self.content.slice_to(self.type_limit), + // self.content.slice(self.type_limit, self.self_limit), + // self.content.slice_from(self.self_limit)) + type_limit: uint, + self_limit: uint, + content: Vec, } impl VecPerParamSpace { pub fn push_all(&mut self, space: ParamSpace, values: &[T]) { - self.get_mut_vec(space).push_all(values); + // FIXME (#15435): slow; O(n^2); could enhance vec to make it O(n). + for t in values.iter() { + self.push(space, t.clone()); + } } } impl VecPerParamSpace { + fn limits(&self, space: ParamSpace) -> (uint, uint) { + match space { + TypeSpace => (0, self.type_limit), + SelfSpace => (self.type_limit, self.self_limit), + FnSpace => (self.self_limit, self.content.len()), + } + } + pub fn empty() -> VecPerParamSpace { VecPerParamSpace { - vecs: (Vec::new(), Vec::new(), Vec::new()) + type_limit: 0, + self_limit: 0, + content: Vec::new() } } @@ -282,8 +306,15 @@ impl VecPerParamSpace { } pub fn new(t: Vec, s: Vec, f: Vec) -> VecPerParamSpace { + let type_limit = t.len(); + let self_limit = t.len() + s.len(); + let mut content = t; + content.push_all_move(s); + content.push_all_move(f); VecPerParamSpace { - vecs: (t, s, f) + type_limit: type_limit, + self_limit: self_limit, + content: content, } } @@ -295,75 +326,98 @@ impl VecPerParamSpace { result } + /// Appends `value` to the vector associated with `space`. + /// + /// Unlike the `push` method in `Vec`, this should not be assumed + /// to be a cheap operation (even when amortized over many calls). pub fn push(&mut self, space: ParamSpace, value: T) { - self.get_mut_vec(space).push(value); + let (_, limit) = self.limits(space); + match space { + TypeSpace => { self.type_limit += 1; self.self_limit += 1; } + SelfSpace => { self.self_limit += 1; } + FnSpace => {} + } + self.content.insert(limit, value); } pub fn pop(&mut self, space: ParamSpace) -> Option { - self.get_mut_vec(space).pop() + let (start, limit) = self.limits(space); + if start == limit { + None + } else { + match space { + TypeSpace => { self.type_limit -= 1; self.self_limit -= 1; } + SelfSpace => { self.self_limit -= 1; } + FnSpace => {} + } + self.content.remove(limit - 1) + } } pub fn truncate(&mut self, space: ParamSpace, len: uint) { - self.get_mut_vec(space).truncate(len) + // FIXME (#15435): slow; O(n^2); could enhance vec to make it O(n). + while self.len(space) > len { + self.pop(space); + } } pub fn replace(&mut self, space: ParamSpace, elems: Vec) { - *self.get_mut_vec(space) = elems; + // FIXME (#15435): slow; O(n^2); could enhance vec to make it O(n). + self.truncate(space, 0); + for t in elems.move_iter() { + self.push(space, t); + } } pub fn get_self<'a>(&'a self) -> Option<&'a T> { - let v = self.get_vec(SelfSpace); + let v = self.get_slice(SelfSpace); assert!(v.len() <= 1); - if v.len() == 0 { None } else { Some(v.get(0)) } + if v.len() == 0 { None } else { Some(&v[0]) } } pub fn len(&self, space: ParamSpace) -> uint { - self.get_vec(space).len() + self.get_slice(space).len() } pub fn is_empty_in(&self, space: ParamSpace) -> bool { - self.get_vec(space).len() == 0 + self.len(space) == 0 } pub fn get_slice<'a>(&'a self, space: ParamSpace) -> &'a [T] { - self.get_vec(space).as_slice() + let (start, limit) = self.limits(space); + self.content.slice(start, limit) } - fn get_vec<'a>(&'a self, space: ParamSpace) -> &'a Vec { - self.vecs.get(space as uint).unwrap() - } - - fn get_mut_vec<'a>(&'a mut self, space: ParamSpace) -> &'a mut Vec { - self.vecs.get_mut(space as uint).unwrap() + fn get_mut_slice<'a>(&'a mut self, space: ParamSpace) -> &'a mut [T] { + let (start, limit) = self.limits(space); + self.content.mut_slice(start, limit) } pub fn opt_get<'a>(&'a self, space: ParamSpace, index: uint) -> Option<&'a T> { - let v = self.get_vec(space); - if index < v.len() { Some(v.get(index)) } else { None } + let v = self.get_slice(space); + if index < v.len() { Some(&v[index]) } else { None } } pub fn get<'a>(&'a self, space: ParamSpace, index: uint) -> &'a T { - self.get_vec(space).get(index) + &self.get_slice(space)[index] } pub fn get_mut<'a>(&'a mut self, space: ParamSpace, index: uint) -> &'a mut T { - self.get_mut_vec(space).get_mut(index) + &mut self.get_mut_slice(space)[index] } - pub fn iter<'a>(&'a self) -> Chain, - Chain, - Items<'a,T>>> { - let (ref r, ref s, ref f) = self.vecs; - r.iter().chain(s.iter().chain(f.iter())) + pub fn iter<'a>(&'a self) -> Items<'a,T> { + self.content.iter() } pub fn all_vecs(&self, pred: |&[T]| -> bool) -> bool { - self.vecs.iter().map(|v|v.as_slice()).all(pred) + let spaces = [TypeSpace, SelfSpace, FnSpace]; + spaces.iter().all(|&space| { pred(self.get_slice(space)) }) } pub fn all(&self, pred: |&T| -> bool) -> bool { @@ -379,9 +433,13 @@ impl VecPerParamSpace { } pub fn map(&self, pred: |&T| -> U) -> VecPerParamSpace { - VecPerParamSpace::new(self.vecs.ref0().iter().map(|p| pred(p)).collect(), - self.vecs.ref1().iter().map(|p| pred(p)).collect(), - self.vecs.ref2().iter().map(|p| pred(p)).collect()) + // FIXME (#15418): this could avoid allocating the intermediate + // Vec's, but note that the values of type_limit and self_limit + // also need to be kept in sync during construction. + VecPerParamSpace::new( + self.get_slice(TypeSpace).iter().map(|p| pred(p)).collect(), + self.get_slice(SelfSpace).iter().map(|p| pred(p)).collect(), + self.get_slice(FnSpace).iter().map(|p| pred(p)).collect()) } pub fn map_rev(&self, pred: |&T| -> U) -> VecPerParamSpace { @@ -394,29 +452,46 @@ impl VecPerParamSpace { * can be run to a fixed point */ - let mut fns: Vec = self.vecs.ref2().iter().rev().map(|p| pred(p)).collect(); + let mut fns: Vec = self.get_slice(FnSpace).iter().rev().map(|p| pred(p)).collect(); // NB: Calling foo.rev().map().rev() causes the calls to map // to occur in the wrong order. This was somewhat surprising // to me, though it makes total sense. fns.reverse(); - let mut selfs: Vec = self.vecs.ref1().iter().rev().map(|p| pred(p)).collect(); + let mut selfs: Vec = self.get_slice(SelfSpace).iter().rev().map(|p| pred(p)).collect(); selfs.reverse(); - let mut tys: Vec = self.vecs.ref0().iter().rev().map(|p| pred(p)).collect(); + let mut tys: Vec = self.get_slice(TypeSpace).iter().rev().map(|p| pred(p)).collect(); tys.reverse(); VecPerParamSpace::new(tys, selfs, fns) } pub fn split(self) -> (Vec, Vec, Vec) { - self.vecs + // FIXME (#15418): this does two traversals when in principle + // one would suffice. i.e. change to use `move_iter`. + let VecPerParamSpace { type_limit, self_limit, content } = self; + let mut i = 0; + let (prefix, fn_vec) = content.partition(|_| { + let on_left = i < self_limit; + i += 1; + on_left + }); + + let mut i = 0; + let (type_vec, self_vec) = prefix.partition(|_| { + let on_left = i < type_limit; + i += 1; + on_left + }); + + (type_vec, self_vec, fn_vec) } pub fn with_vec(mut self, space: ParamSpace, vec: Vec) -> VecPerParamSpace { - assert!(self.get_vec(space).is_empty()); - *self.get_mut_vec(space) = vec; + assert!(self.is_empty_in(space)); + self.replace(space, vec); self } }