Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 82d8833

Browse files
authoredNov 21, 2016
Auto merge of #37642 - nnethercote:no-HirVec-of-P, r=michaelwoerister
Change HirVec<P<T>> to HirVec<T> in hir:: Expr. This PR changes data structures like this: ``` [ ExprArray | 8 | P ] | v [ P | P | P | P | P | P | P | P ] | v [ ExprTup | 2 | P ] | v [ P | P ] | v [ Expr ] ``` to this: ``` [ ExprArray | 8 | P ] | v [ [ ExprTup | 2 | P ] | ... ] | v [ Expr | Expr ] ``` I thought this would be a win for #36799, and on a cut-down version of that workload this reduces the peak heap size (as measured by Massif) from 885 MiB to 875 MiB. However, the peak RSS as measured by `-Ztime-passes` and by `/usr/bin/time` increases by about 30 MiB. I'm not sure why. Just look at the picture above -- the second data structure clearly takes up less space than the first. My best idea relates to unused elements in the slices. `HirVec<Expr>` is a typedef for `P<[Expr]>`. If there were any unused excess elements then I could see that memory usage would increase, because those excess elements are larger in `HirVec<Expr>` than in `HirVec<P<Expr>>`. But AIUI there are no such excess elements, and Massif's measurements corroborate that. However, the two main creation points for these data structures are these lines from `lower_expr`: ```rust ExprKind::Vec(ref exprs) => { hir::ExprArray(exprs.iter().map(|x| self.lower_expr(x)).collect()) } ExprKind::Tup(ref elts) => { hir::ExprTup(elts.iter().map(|x| self.lower_expr(x)).collect()) } ``` I suspect what is happening is that temporary excess elements are created within the `collect` calls. The workload from #36799 has many 2-tuples and 3-tuples and when `Vec` gets doubled it goes from a capacity of 1 to 4, which would lead to excess elements. Though, having said that, `Vec::with_capacity` doesn't create excess AFAICT. So I'm not sure. What goes on inside `collect` is complex. Anyway, in its current form this PR is a memory consumption regression and so not worth landing but I figured I'd post it in case anyone has additional insight.
2 parents 7b3eeea + 382d3b0 commit 82d8833

File tree

10 files changed

+183
-183
lines changed

10 files changed

+183
-183
lines changed
 

‎src/librustc/cfg/construct.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -299,15 +299,15 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
299299
}
300300

301301
hir::ExprArray(ref elems) => {
302-
self.straightline(expr, pred, elems.iter().map(|e| &**e))
302+
self.straightline(expr, pred, elems.iter().map(|e| &*e))
303303
}
304304

305305
hir::ExprCall(ref func, ref args) => {
306-
self.call(expr, pred, &func, args.iter().map(|e| &**e))
306+
self.call(expr, pred, &func, args.iter().map(|e| &*e))
307307
}
308308

309309
hir::ExprMethodCall(.., ref args) => {
310-
self.call(expr, pred, &args[0], args[1..].iter().map(|e| &**e))
310+
self.call(expr, pred, &args[0], args[1..].iter().map(|e| &*e))
311311
}
312312

313313
hir::ExprIndex(ref l, ref r) |
@@ -320,7 +320,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
320320
}
321321

322322
hir::ExprTup(ref exprs) => {
323-
self.straightline(expr, pred, exprs.iter().map(|e| &**e))
323+
self.straightline(expr, pred, exprs.iter().map(|e| &*e))
324324
}
325325

326326
hir::ExprStruct(_, ref fields, ref base) => {
@@ -353,8 +353,8 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
353353
}
354354

355355
hir::ExprInlineAsm(_, ref outputs, ref inputs) => {
356-
let post_outputs = self.exprs(outputs.iter().map(|e| &**e), pred);
357-
let post_inputs = self.exprs(inputs.iter().map(|e| &**e), post_outputs);
356+
let post_outputs = self.exprs(outputs.iter().map(|e| &*e), pred);
357+
let post_inputs = self.exprs(inputs.iter().map(|e| &*e), post_outputs);
358358
self.add_ast_node(expr.id, &[post_inputs])
359359
}
360360

‎src/librustc/hir/lowering.rs

+140-138
Large diffs are not rendered by default.

‎src/librustc/hir/mod.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -867,12 +867,12 @@ pub enum Expr_ {
867867
/// A `box x` expression.
868868
ExprBox(P<Expr>),
869869
/// An array (`[a, b, c, d]`)
870-
ExprArray(HirVec<P<Expr>>),
870+
ExprArray(HirVec<Expr>),
871871
/// A function call
872872
///
873873
/// The first field resolves to the function itself (usually an `ExprPath`),
874874
/// and the second field is the list of arguments
875-
ExprCall(P<Expr>, HirVec<P<Expr>>),
875+
ExprCall(P<Expr>, HirVec<Expr>),
876876
/// A method call (`x.foo::<Bar, Baz>(a, b, c, d)`)
877877
///
878878
/// The `Spanned<Name>` is the identifier for the method name.
@@ -885,9 +885,9 @@ pub enum Expr_ {
885885
///
886886
/// Thus, `x.foo::<Bar, Baz>(a, b, c, d)` is represented as
887887
/// `ExprMethodCall(foo, [Bar, Baz], [x, a, b, c, d])`.
888-
ExprMethodCall(Spanned<Name>, HirVec<P<Ty>>, HirVec<P<Expr>>),
888+
ExprMethodCall(Spanned<Name>, HirVec<P<Ty>>, HirVec<Expr>),
889889
/// A tuple (`(a, b, c ,d)`)
890-
ExprTup(HirVec<P<Expr>>),
890+
ExprTup(HirVec<Expr>),
891891
/// A binary operation (For example: `a + b`, `a * b`)
892892
ExprBinary(BinOp, P<Expr>, P<Expr>),
893893
/// A unary operation (For example: `!x`, `*x`)
@@ -951,7 +951,7 @@ pub enum Expr_ {
951951
ExprRet(Option<P<Expr>>),
952952

953953
/// Inline assembly (from `asm!`), with its outputs and inputs.
954-
ExprInlineAsm(P<InlineAsm>, HirVec<P<Expr>>, HirVec<P<Expr>>),
954+
ExprInlineAsm(P<InlineAsm>, HirVec<Expr>, HirVec<Expr>),
955955

956956
/// A struct or struct-like variant literal expression.
957957
///

‎src/librustc/hir/print.rs

+12-12
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ impl<'a> State<'a> {
452452
self.end()
453453
}
454454

455-
pub fn commasep_exprs(&mut self, b: Breaks, exprs: &[P<hir::Expr>]) -> io::Result<()> {
455+
pub fn commasep_exprs(&mut self, b: Breaks, exprs: &[hir::Expr]) -> io::Result<()> {
456456
self.commasep_cmnt(b, exprs, |s, e| s.print_expr(&e), |e| e.span)
457457
}
458458

@@ -1200,7 +1200,7 @@ impl<'a> State<'a> {
12001200
}
12011201

12021202

1203-
fn print_call_post(&mut self, args: &[P<hir::Expr>]) -> io::Result<()> {
1203+
fn print_call_post(&mut self, args: &[hir::Expr]) -> io::Result<()> {
12041204
self.popen()?;
12051205
self.commasep_exprs(Inconsistent, args)?;
12061206
self.pclose()
@@ -1218,10 +1218,10 @@ impl<'a> State<'a> {
12181218
Ok(())
12191219
}
12201220

1221-
fn print_expr_vec(&mut self, exprs: &[P<hir::Expr>]) -> io::Result<()> {
1221+
fn print_expr_vec(&mut self, exprs: &[hir::Expr]) -> io::Result<()> {
12221222
self.ibox(indent_unit)?;
12231223
word(&mut self.s, "[")?;
1224-
self.commasep_exprs(Inconsistent, &exprs[..])?;
1224+
self.commasep_exprs(Inconsistent, exprs)?;
12251225
word(&mut self.s, "]")?;
12261226
self.end()
12271227
}
@@ -1274,24 +1274,24 @@ impl<'a> State<'a> {
12741274
Ok(())
12751275
}
12761276

1277-
fn print_expr_tup(&mut self, exprs: &[P<hir::Expr>]) -> io::Result<()> {
1277+
fn print_expr_tup(&mut self, exprs: &[hir::Expr]) -> io::Result<()> {
12781278
self.popen()?;
1279-
self.commasep_exprs(Inconsistent, &exprs[..])?;
1279+
self.commasep_exprs(Inconsistent, exprs)?;
12801280
if exprs.len() == 1 {
12811281
word(&mut self.s, ",")?;
12821282
}
12831283
self.pclose()
12841284
}
12851285

1286-
fn print_expr_call(&mut self, func: &hir::Expr, args: &[P<hir::Expr>]) -> io::Result<()> {
1286+
fn print_expr_call(&mut self, func: &hir::Expr, args: &[hir::Expr]) -> io::Result<()> {
12871287
self.print_expr_maybe_paren(func)?;
12881288
self.print_call_post(args)
12891289
}
12901290

12911291
fn print_expr_method_call(&mut self,
12921292
name: Spanned<ast::Name>,
12931293
tys: &[P<hir::Ty>],
1294-
args: &[P<hir::Expr>])
1294+
args: &[hir::Expr])
12951295
-> io::Result<()> {
12961296
let base_args = &args[1..];
12971297
self.print_expr(&args[0])?;
@@ -1340,7 +1340,7 @@ impl<'a> State<'a> {
13401340
self.print_expr(expr)?;
13411341
}
13421342
hir::ExprArray(ref exprs) => {
1343-
self.print_expr_vec(&exprs[..])?;
1343+
self.print_expr_vec(exprs)?;
13441344
}
13451345
hir::ExprRepeat(ref element, ref count) => {
13461346
self.print_expr_repeat(&element, &count)?;
@@ -1349,13 +1349,13 @@ impl<'a> State<'a> {
13491349
self.print_expr_struct(path, &fields[..], wth)?;
13501350
}
13511351
hir::ExprTup(ref exprs) => {
1352-
self.print_expr_tup(&exprs[..])?;
1352+
self.print_expr_tup(exprs)?;
13531353
}
13541354
hir::ExprCall(ref func, ref args) => {
1355-
self.print_expr_call(&func, &args[..])?;
1355+
self.print_expr_call(&func, args)?;
13561356
}
13571357
hir::ExprMethodCall(name, ref tys, ref args) => {
1358-
self.print_expr_method_call(name, &tys[..], &args[..])?;
1358+
self.print_expr_method_call(name, &tys[..], args)?;
13591359
}
13601360
hir::ExprBinary(op, ref lhs, ref rhs) => {
13611361
self.print_expr_binary(op, &lhs, &rhs)?;

‎src/librustc/middle/expr_use_visitor.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
327327
self.delegate.consume(consume_id, consume_span, cmt, mode);
328328
}
329329

330-
fn consume_exprs(&mut self, exprs: &[P<hir::Expr>]) {
330+
fn consume_exprs(&mut self, exprs: &[hir::Expr]) {
331331
for expr in exprs {
332332
self.consume_expr(&expr);
333333
}

‎src/librustc/middle/liveness.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ use std::io::prelude::*;
123123
use std::io;
124124
use std::rc::Rc;
125125
use syntax::ast::{self, NodeId};
126-
use syntax::ptr::P;
127126
use syntax::symbol::keywords;
128127
use syntax_pos::Span;
129128

@@ -902,7 +901,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
902901
self.define_bindings_in_pat(&local.pat, succ)
903902
}
904903

905-
fn propagate_through_exprs(&mut self, exprs: &[P<Expr>], succ: LiveNode)
904+
fn propagate_through_exprs(&mut self, exprs: &[Expr], succ: LiveNode)
906905
-> LiveNode {
907906
exprs.iter().rev().fold(succ, |succ, expr| {
908907
self.propagate_through_expr(&expr, succ)
@@ -1087,7 +1086,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
10871086
// Uninteresting cases: just propagate in rev exec order
10881087

10891088
hir::ExprArray(ref exprs) => {
1090-
self.propagate_through_exprs(&exprs[..], succ)
1089+
self.propagate_through_exprs(exprs, succ)
10911090
}
10921091

10931092
hir::ExprRepeat(ref element, ref count) => {
@@ -1111,7 +1110,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
11111110
} else {
11121111
succ
11131112
};
1114-
let succ = self.propagate_through_exprs(&args[..], succ);
1113+
let succ = self.propagate_through_exprs(args, succ);
11151114
self.propagate_through_expr(&f, succ)
11161115
}
11171116

@@ -1124,11 +1123,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
11241123
} else {
11251124
succ
11261125
};
1127-
self.propagate_through_exprs(&args[..], succ)
1126+
self.propagate_through_exprs(args, succ)
11281127
}
11291128

11301129
hir::ExprTup(ref exprs) => {
1131-
self.propagate_through_exprs(&exprs[..], succ)
1130+
self.propagate_through_exprs(exprs, succ)
11321131
}
11331132

11341133
hir::ExprBinary(op, ref l, ref r) if op.node.is_lazy() => {

‎src/librustc_const_eval/eval.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ pub fn const_expr_to_pat<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
297297
_ => bug!()
298298
};
299299
let pats = args.iter()
300-
.map(|expr| const_expr_to_pat(tcx, &**expr, pat_id, span))
300+
.map(|expr| const_expr_to_pat(tcx, &*expr, pat_id, span))
301301
.collect::<Result<_, _>>()?;
302302
PatKind::TupleStruct(path, pats, None)
303303
}

‎src/librustc_typeck/check/callee.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use hir::print;
1717
use rustc::{infer, traits};
1818
use rustc::ty::{self, LvaluePreference, Ty};
1919
use syntax::symbol::Symbol;
20-
use syntax::ptr::P;
2120
use syntax_pos::Span;
2221

2322
use rustc::hir;
@@ -46,7 +45,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
4645
pub fn check_call(&self,
4746
call_expr: &'gcx hir::Expr,
4847
callee_expr: &'gcx hir::Expr,
49-
arg_exprs: &'gcx [P<hir::Expr>],
48+
arg_exprs: &'gcx [hir::Expr],
5049
expected: Expectation<'tcx>)
5150
-> Ty<'tcx> {
5251
let original_callee_ty = self.check_expr(callee_expr);
@@ -189,7 +188,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
189188
fn confirm_builtin_call(&self,
190189
call_expr: &hir::Expr,
191190
callee_ty: Ty<'tcx>,
192-
arg_exprs: &'gcx [P<hir::Expr>],
191+
arg_exprs: &'gcx [hir::Expr],
193192
expected: Expectation<'tcx>)
194193
-> Ty<'tcx> {
195194
let error_fn_sig;
@@ -272,7 +271,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
272271

273272
fn confirm_deferred_closure_call(&self,
274273
call_expr: &hir::Expr,
275-
arg_exprs: &'gcx [P<hir::Expr>],
274+
arg_exprs: &'gcx [hir::Expr],
276275
expected: Expectation<'tcx>,
277276
fn_sig: ty::FnSig<'tcx>)
278277
-> Ty<'tcx> {
@@ -299,7 +298,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
299298
fn confirm_overloaded_call(&self,
300299
call_expr: &hir::Expr,
301300
callee_expr: &'gcx hir::Expr,
302-
arg_exprs: &'gcx [P<hir::Expr>],
301+
arg_exprs: &'gcx [hir::Expr],
303302
expected: Expectation<'tcx>,
304303
method_callee: ty::MethodCallee<'tcx>)
305304
-> Ty<'tcx> {

‎src/librustc_typeck/check/mod.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -2409,7 +2409,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
24092409
sp: Span,
24102410
method_fn_ty: Ty<'tcx>,
24112411
callee_expr: &'gcx hir::Expr,
2412-
args_no_rcvr: &'gcx [P<hir::Expr>],
2412+
args_no_rcvr: &'gcx [hir::Expr],
24132413
tuple_arguments: TupleArgumentsFlag,
24142414
expected: Expectation<'tcx>)
24152415
-> Ty<'tcx> {
@@ -2448,7 +2448,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
24482448
sp: Span,
24492449
fn_inputs: &[Ty<'tcx>],
24502450
expected_arg_tys: &[Ty<'tcx>],
2451-
args: &'gcx [P<hir::Expr>],
2451+
args: &'gcx [hir::Expr],
24522452
variadic: bool,
24532453
tuple_arguments: TupleArgumentsFlag) {
24542454
let tcx = self.tcx;
@@ -2822,7 +2822,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
28222822
fn check_method_call(&self,
28232823
expr: &'gcx hir::Expr,
28242824
method_name: Spanned<ast::Name>,
2825-
args: &'gcx [P<hir::Expr>],
2825+
args: &'gcx [hir::Expr],
28262826
tps: &[P<hir::Ty>],
28272827
expected: Expectation<'tcx>,
28282828
lvalue_pref: LvaluePreference) -> Ty<'tcx> {
@@ -3670,10 +3670,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
36703670
self.check_block_with_expected(&b, expected)
36713671
}
36723672
hir::ExprCall(ref callee, ref args) => {
3673-
self.check_call(expr, &callee, &args[..], expected)
3673+
self.check_call(expr, &callee, args, expected)
36743674
}
36753675
hir::ExprMethodCall(name, ref tps, ref args) => {
3676-
self.check_method_call(expr, name, &args[..], &tps[..], expected, lvalue_pref)
3676+
self.check_method_call(expr, name, args, &tps[..], expected, lvalue_pref)
36773677
}
36783678
hir::ExprCast(ref e, ref t) => {
36793679
if let hir::TyArray(_, ref count_expr) = t.node {
@@ -3728,7 +3728,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
37283728
let result = if i == 0 {
37293729
self.try_coerce(e, e_ty, coerce_to)
37303730
} else {
3731-
let prev_elems = || args[..i].iter().map(|e| &**e);
3731+
let prev_elems = || args[..i].iter().map(|e| &*e);
37323732
self.try_find_coercion_lub(&cause, prev_elems, unified, e, e_ty)
37333733
};
37343734

‎src/librustc_typeck/check/regionck.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -613,19 +613,19 @@ impl<'a, 'gcx, 'tcx, 'v> Visitor<'v> for RegionCtxt<'a, 'gcx, 'tcx> {
613613
hir::ExprCall(ref callee, ref args) => {
614614
if has_method_map {
615615
self.constrain_call(expr, Some(&callee),
616-
args.iter().map(|e| &**e), false);
616+
args.iter().map(|e| &*e), false);
617617
} else {
618618
self.constrain_callee(callee.id, expr, &callee);
619619
self.constrain_call(expr, None,
620-
args.iter().map(|e| &**e), false);
620+
args.iter().map(|e| &*e), false);
621621
}
622622

623623
intravisit::walk_expr(self, expr);
624624
}
625625

626626
hir::ExprMethodCall(.., ref args) => {
627627
self.constrain_call(expr, Some(&args[0]),
628-
args[1..].iter().map(|e| &**e), false);
628+
args[1..].iter().map(|e| &*e), false);
629629

630630
intravisit::walk_expr(self, expr);
631631
}

0 commit comments

Comments
 (0)
Failed to load comments.