Skip to content

Commit

Permalink
Avoid many cmt allocations.
Browse files Browse the repository at this point in the history
`cmt` is a ref-counted wrapper around `cmt_` The use of refcounting
keeps `cmt` handling simple, but a lot of `cmt` instances are very
short-lived, and heap-allocating the short-lived ones takes up time.

This patch changes things in the following ways.

- Most of the functions that produced `cmt` instances now produce `cmt_`
  instances. The `Rc::new` calls that occurred within those functions
  now occur at their call sites (but only when necessary, which isn't
  that often).

- Many of the functions that took `cmt` arguments now take `&cmt_`
  arguments. This includes all the methods in the `Delegate` trait.

As a result, the vast majority of the heap allocations are avoided. In
an extreme case, the number of calls to malloc in tuple-stress drops
from 9.9M to 7.9M, a drop of 20%. And the compile times for many runs of
coercions, deep-vector, and tuple-stress drop by 1--2%.
  • Loading branch information
nnethercote committed May 3, 2018
1 parent f76f6fb commit 7cf142f
Show file tree
Hide file tree
Showing 12 changed files with 208 additions and 209 deletions.
60 changes: 30 additions & 30 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use ty::{self, TyCtxt, adjustment};

use hir::{self, PatKind};
use rustc_data_structures::sync::Lrc;
use std::rc::Rc;
use syntax::ast;
use syntax::ptr::P;
use syntax_pos::Span;
Expand All @@ -44,7 +45,7 @@ pub trait Delegate<'tcx> {
fn consume(&mut self,
consume_id: ast::NodeId,
consume_span: Span,
cmt: mc::cmt<'tcx>,
cmt: &mc::cmt_<'tcx>,
mode: ConsumeMode);

// The value found at `cmt` has been determined to match the
Expand All @@ -61,22 +62,22 @@ pub trait Delegate<'tcx> {
// called on a subpart of an input passed to `matched_pat).
fn matched_pat(&mut self,
matched_pat: &hir::Pat,
cmt: mc::cmt<'tcx>,
cmt: &mc::cmt_<'tcx>,
mode: MatchMode);

// The value found at `cmt` is either copied or moved via the
// pattern binding `consume_pat`, depending on mode.
fn consume_pat(&mut self,
consume_pat: &hir::Pat,
cmt: mc::cmt<'tcx>,
cmt: &mc::cmt_<'tcx>,
mode: ConsumeMode);

// The value found at `borrow` is being borrowed at the point
// `borrow_id` for the region `loan_region` with kind `bk`.
fn borrow(&mut self,
borrow_id: ast::NodeId,
borrow_span: Span,
cmt: mc::cmt<'tcx>,
cmt: &mc::cmt_<'tcx>,
loan_region: ty::Region<'tcx>,
bk: ty::BorrowKind,
loan_cause: LoanCause);
Expand All @@ -90,7 +91,7 @@ pub trait Delegate<'tcx> {
fn mutate(&mut self,
assignment_id: ast::NodeId,
assignment_span: Span,
assignee_cmt: mc::cmt<'tcx>,
assignee_cmt: &mc::cmt_<'tcx>,
mode: MutateMode);
}

Expand Down Expand Up @@ -316,11 +317,11 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {

let fn_body_scope_r =
self.tcx().mk_region(ty::ReScope(region::Scope::Node(body.value.hir_id.local_id)));
let arg_cmt = self.mc.cat_rvalue(
let arg_cmt = Rc::new(self.mc.cat_rvalue(
arg.id,
arg.pat.span,
fn_body_scope_r, // Args live only as long as the fn body.
arg_ty);
arg_ty));

self.walk_irrefutable_pat(arg_cmt, &arg.pat);
}
Expand All @@ -335,11 +336,11 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
fn delegate_consume(&mut self,
consume_id: ast::NodeId,
consume_span: Span,
cmt: mc::cmt<'tcx>) {
cmt: &mc::cmt_<'tcx>) {
debug!("delegate_consume(consume_id={}, cmt={:?})",
consume_id, cmt);

let mode = copy_or_move(&self.mc, self.param_env, &cmt, DirectRefMove);
let mode = copy_or_move(&self.mc, self.param_env, cmt, DirectRefMove);
self.delegate.consume(consume_id, consume_span, cmt, mode);
}

Expand All @@ -353,7 +354,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
debug!("consume_expr(expr={:?})", expr);

let cmt = return_if_err!(self.mc.cat_expr(expr));
self.delegate_consume(expr.id, expr.span, cmt);
self.delegate_consume(expr.id, expr.span, &cmt);
self.walk_expr(expr);
}

Expand All @@ -362,7 +363,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
expr: &hir::Expr,
mode: MutateMode) {
let cmt = return_if_err!(self.mc.cat_expr(expr));
self.delegate.mutate(assignment_expr.id, assignment_expr.span, cmt, mode);
self.delegate.mutate(assignment_expr.id, assignment_expr.span, &cmt, mode);
self.walk_expr(expr);
}

Expand All @@ -375,7 +376,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
expr, r, bk);

let cmt = return_if_err!(self.mc.cat_expr(expr));
self.delegate.borrow(expr.id, expr.span, cmt, r, bk, cause);
self.delegate.borrow(expr.id, expr.span, &cmt, r, bk, cause);

self.walk_expr(expr)
}
Expand Down Expand Up @@ -435,7 +436,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
}

hir::ExprMatch(ref discr, ref arms, _) => {
let discr_cmt = return_if_err!(self.mc.cat_expr(&discr));
let discr_cmt = Rc::new(return_if_err!(self.mc.cat_expr(&discr)));
let r = self.tcx().types.re_empty;
self.borrow_expr(&discr, r, ty::ImmBorrow, MatchDiscriminant);

Expand Down Expand Up @@ -619,7 +620,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
// "assigns", which is handled by
// `walk_pat`:
self.walk_expr(&expr);
let init_cmt = return_if_err!(self.mc.cat_expr(&expr));
let init_cmt = Rc::new(return_if_err!(self.mc.cat_expr(&expr)));
self.walk_irrefutable_pat(init_cmt, &local.pat);
}
}
Expand Down Expand Up @@ -652,7 +653,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
None => { return; }
};

let with_cmt = return_if_err!(self.mc.cat_expr(&with_expr));
let with_cmt = Rc::new(return_if_err!(self.mc.cat_expr(&with_expr)));

// Select just those fields of the `with`
// expression that will actually be used
Expand All @@ -671,7 +672,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
with_field.name,
with_field.ty(self.tcx(), substs)
);
self.delegate_consume(with_expr.id, with_expr.span, cmt_field);
self.delegate_consume(with_expr.id, with_expr.span, &cmt_field);
}
}
}
Expand Down Expand Up @@ -710,7 +711,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
adjustment::Adjust::Unsize => {
// Creating a closure/fn-pointer or unsizing consumes
// the input and stores it into the resulting rvalue.
self.delegate_consume(expr.id, expr.span, cmt.clone());
self.delegate_consume(expr.id, expr.span, &cmt);
}

adjustment::Adjust::Deref(None) => {}
Expand All @@ -722,12 +723,11 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
// this is an autoref of `x`.
adjustment::Adjust::Deref(Some(ref deref)) => {
let bk = ty::BorrowKind::from_mutbl(deref.mutbl);
self.delegate.borrow(expr.id, expr.span, cmt.clone(),
deref.region, bk, AutoRef);
self.delegate.borrow(expr.id, expr.span, &cmt, deref.region, bk, AutoRef);
}

adjustment::Adjust::Borrow(ref autoref) => {
self.walk_autoref(expr, cmt.clone(), autoref);
self.walk_autoref(expr, &cmt, autoref);
}
}
cmt = return_if_err!(self.mc.cat_expr_adjusted(expr, cmt, &adjustment));
Expand All @@ -739,7 +739,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
/// after all relevant autoderefs have occurred.
fn walk_autoref(&mut self,
expr: &hir::Expr,
cmt_base: mc::cmt<'tcx>,
cmt_base: &mc::cmt_<'tcx>,
autoref: &adjustment::AutoBorrow<'tcx>) {
debug!("walk_autoref(expr.id={} cmt_base={:?} autoref={:?})",
expr.id,
Expand Down Expand Up @@ -852,7 +852,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
// Each match binding is effectively an assignment to the
// binding being produced.
let def = Def::Local(canonical_id);
if let Ok(binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty, def) {
if let Ok(ref binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty, def) {
delegate.mutate(pat.id, pat.span, binding_cmt, MutateMode::Init);
}

Expand All @@ -861,13 +861,13 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
ty::BindByReference(m) => {
if let ty::TyRef(r, _) = pat_ty.sty {
let bk = ty::BorrowKind::from_mutbl(m);
delegate.borrow(pat.id, pat.span, cmt_pat, r, bk, RefBinding);
delegate.borrow(pat.id, pat.span, &cmt_pat, r, bk, RefBinding);
}
}
ty::BindByValue(..) => {
let mode = copy_or_move(mc, param_env, &cmt_pat, PatBindingMove);
debug!("walk_pat binding consuming pat");
delegate.consume_pat(pat, cmt_pat, mode);
delegate.consume_pat(pat, &cmt_pat, mode);
}
}
}
Expand All @@ -891,12 +891,12 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
let downcast_cmt = mc.cat_downcast_if_needed(pat, cmt_pat, variant_did);

debug!("variant downcast_cmt={:?} pat={:?}", downcast_cmt, pat);
delegate.matched_pat(pat, downcast_cmt, match_mode);
delegate.matched_pat(pat, &downcast_cmt, match_mode);
}
Def::Struct(..) | Def::StructCtor(..) | Def::Union(..) |
Def::TyAlias(..) | Def::AssociatedTy(..) | Def::SelfTy(..) => {
debug!("struct cmt_pat={:?} pat={:?}", cmt_pat, pat);
delegate.matched_pat(pat, cmt_pat, match_mode);
delegate.matched_pat(pat, &cmt_pat, match_mode);
}
_ => {}
}
Expand Down Expand Up @@ -924,12 +924,12 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
self.param_env,
&cmt_var,
CaptureMove);
self.delegate.consume(closure_expr.id, freevar.span, cmt_var, mode);
self.delegate.consume(closure_expr.id, freevar.span, &cmt_var, mode);
}
ty::UpvarCapture::ByRef(upvar_borrow) => {
self.delegate.borrow(closure_expr.id,
fn_decl_span,
cmt_var,
&cmt_var,
upvar_borrow.region,
upvar_borrow.kind,
ClosureCapture(freevar.span));
Expand All @@ -943,7 +943,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
closure_id: ast::NodeId,
closure_span: Span,
upvar: &hir::Freevar)
-> mc::McResult<mc::cmt<'tcx>> {
-> mc::McResult<mc::cmt_<'tcx>> {
// Create the cmt for the variable being borrowed, from the
// caller's perspective
let var_hir_id = self.tcx().hir.node_to_hir_id(upvar.var_id());
Expand All @@ -954,7 +954,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {

fn copy_or_move<'a, 'gcx, 'tcx>(mc: &mc::MemCategorizationContext<'a, 'gcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
cmt: &mc::cmt<'tcx>,
cmt: &mc::cmt_<'tcx>,
move_reason: MoveReason)
-> ConsumeMode
{
Expand Down
Loading

0 comments on commit 7cf142f

Please sign in to comment.