Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collect statistics about MIR optimizations #76769

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub mod fingerprint;
pub mod profiling;
pub mod sharded;
pub mod stack;
pub mod statistics;
pub mod sync;
pub mod thin_vec;
pub mod tiny_list;
Expand Down
165 changes: 165 additions & 0 deletions compiler/rustc_data_structures/src/statistics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
//! # Support for collecting simple statistics
//!
//! Statistics are useful for collecting metrics from optimization passes, like
//! the number of simplifications performed. To avoid introducing overhead, the
//! collection of statistics is enabled only when rustc is compiled with
//! debug-assertions.
//!
//! Statistics are static variables defined in the module they are used, and
//! lazy registered in the global collector on the first use. Once registered,
//! the collector will obtain their values at the end of compilation process
//! when requested with -Zmir-opt-stats option.

use parking_lot::{const_mutex, Mutex};
use std::io::{self, stdout, Write as _};
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};

static COLLECTOR: Collector = Collector::new();
Copy link
Member

@bjorn3 bjorn3 Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be part of Session or TyCtxt. There may be multiple rustc sessions running in a single process. (for example when using RLS)

Copy link
Contributor Author

@tmiasko tmiasko Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I experimented with different implementation approaches:

  1. Statistics definitions are in the context together with their values.
  2. Statistics definitions are in MIR passes, their values are stored in the context.
  3. Statistics definitions and values are stored together with MIR passes.

I strongly prefer variants that put statistics definitions together with MIR passes that uses them. In that approach, adding and removing counters is trivial, additionally any unused counters are detected as such during compilation.

Values can be stored in the context if we want to support multiple rustc sessions in a single process, although it requires passing context to all those places where counters are used. But is there any real use-case? Why would you use rustc with debug-assertions and MIR optimizations counters inside the RLS?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @bjorn3. We've tried pretty hard to move these sorts of things into Session and TyCtxt to allow running multiple sessions in the same process (both for RLS and eventually for one rustc process compiling multiple crates simultaneously).

Since this is essentially unstable, diagnostic data and doesn't effect the compilation artifacts, it may be worth bending that rule because, as you point out, having the counters defined in their MIR passes is quite nice. I don't personally feel comfortable approving that since this is a cross-cutting concern for the compiler as a whole and there may be others on the compiler team that have strong opinions about breaking that rule.


/// Enables the collection of statistics.
/// To be effective it has to be called before the first use of a statistics.
pub fn try_enable() -> Result<(), ()> {
COLLECTOR.try_enable()
}

/// Prints all statistics collected so far.
pub fn print() {
COLLECTOR.print();
}

pub struct Statistic {
category: &'static str,
name: &'static str,
initialized: AtomicBool,
value: AtomicUsize,
}

struct Collector(Mutex<State>);

struct State {
enabled: bool,
stats: Vec<&'static Statistic>,
}

#[derive(Eq, PartialEq, Ord, PartialOrd)]
struct Snapshot {
category: &'static str,
name: &'static str,
value: usize,
}

impl Statistic {
pub const fn new(category: &'static str, name: &'static str) -> Self {
Statistic {
category,
name,
initialized: AtomicBool::new(false),
value: AtomicUsize::new(0),
}
}

pub fn name(&self) -> &'static str {
self.name
}

pub fn category(&self) -> &'static str {
self.category.rsplit("::").next().unwrap()
}

#[inline]
pub fn register(&'static self) {
if cfg!(debug_assertions) {
if !self.initialized.load(Ordering::Acquire) {
COLLECTOR.register(self);
}
}
}

#[inline]
pub fn increment(&'static self, value: usize) {
if cfg!(debug_assertions) {
self.value.fetch_add(value, Ordering::Relaxed);
tmiasko marked this conversation as resolved.
Show resolved Hide resolved
self.register();
}
}

#[inline]
pub fn update_max(&'static self, value: usize) {
if cfg!(debug_assertions) {
self.value.fetch_max(value, Ordering::Relaxed);
self.register();
}
}

fn snapshot(&'static self) -> Snapshot {
Snapshot {
name: self.name(),
category: self.category(),
value: self.value.load(Ordering::Relaxed),
}
}
}

impl Collector {
const fn new() -> Self {
Collector(const_mutex(State { enabled: false, stats: Vec::new() }))
}

fn try_enable(&self) -> Result<(), ()> {
if cfg!(debug_assertions) {
self.0.lock().enabled = true;
Ok(())
} else {
Err(())
}
}

fn snapshot(&self) -> Vec<Snapshot> {
self.0.lock().stats.iter().copied().map(Statistic::snapshot).collect()
}

fn register(&self, s: &'static Statistic) {
let mut state = self.0.lock();
if !s.initialized.load(Ordering::Relaxed) {
if state.enabled {
state.stats.push(s);
}
s.initialized.store(true, Ordering::Release);
}
}

fn print(&self) {
let mut stats = self.snapshot();
stats.sort();
match self.write(&stats) {
Ok(_) => {}
Err(e) if e.kind() == io::ErrorKind::BrokenPipe => {}
Err(e) => panic!(e),
}
}

fn write(&self, stats: &[Snapshot]) -> io::Result<()> {
let mut cat_width = 0;
let mut val_width = 0;

for s in stats {
cat_width = cat_width.max(s.category.len());
val_width = val_width.max(s.value.to_string().len());
}

let mut out = Vec::new();
for s in stats {
write!(
&mut out,
"{val:val_width$} {cat:cat_width$} {name}\n",
val = s.value,
val_width = val_width,
cat = s.category,
cat_width = cat_width,
name = s.name,
)?;
}

stdout().write_all(&out)
}
}
5 changes: 5 additions & 0 deletions compiler/rustc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ fn run_compiler(

interface::run_compiler(config, |compiler| {
let sess = compiler.session();

let should_stop = RustcDefaultCalls::print_crate_info(
&***compiler.codegen_backend(),
sess,
Expand Down Expand Up @@ -453,6 +454,10 @@ fn run_compiler(
linker.link()?
}

if sess.opts.debugging_opts.mir_opt_stats {
rustc_data_structures::statistics::print();
}

if sess.opts.debugging_opts.perf_stats {
sess.print_perf_stats();
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_mir/src/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::cell::Cell;

use rustc_ast::Mutability;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::statistics::Statistic;
use rustc_hir::def::DefKind;
use rustc_hir::HirId;
use rustc_index::bit_set::BitSet;
Expand Down Expand Up @@ -59,6 +60,10 @@ macro_rules! throw_machine_stop_str {

pub struct ConstProp;

static NUM_PROPAGATED: Statistic = Statistic::new(module_path!(), "Number of const propagations");
static NUM_VALIDATION_ERRORS: Statistic =
Statistic::new(module_path!(), "Number of validation errors during const propagation");

impl<'tcx> MirPass<'tcx> for ConstProp {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
// will be evaluated by miri and produce its errors there
Expand Down Expand Up @@ -622,6 +627,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
ScalarMaybeUninit::Scalar(scalar),
)) = *value
{
NUM_PROPAGATED.increment(1);
*operand = self.operand_from_scalar(
scalar,
value.layout.ty,
Expand Down Expand Up @@ -809,6 +815,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
/*may_ref_to_static*/ true,
) {
trace!("validation error, attempt failed: {:?}", e);
NUM_VALIDATION_ERRORS.increment(1);
return;
}

Expand All @@ -818,6 +825,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
if let Some(Ok(imm)) = imm {
match *imm {
interpret::Immediate::Scalar(ScalarMaybeUninit::Scalar(scalar)) => {
NUM_PROPAGATED.increment(1);
*rval = Rvalue::Use(self.operand_from_scalar(
scalar,
value.layout.ty,
Expand Down Expand Up @@ -859,6 +867,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
if let Some(Some(alloc)) = alloc {
// Assign entire constant in a single statement.
// We can't use aggregates, as we run after the aggregate-lowering `MirPhase`.
NUM_PROPAGATED.increment(1);
*rval = Rvalue::Use(Operand::Constant(Box::new(Constant {
span: source_info.span,
user_ty: None,
Expand Down
24 changes: 22 additions & 2 deletions compiler/rustc_mir/src/transform/copy_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

use crate::transform::MirPass;
use crate::util::def_use::DefUseAnalysis;
use rustc_data_structures::statistics::Statistic;
use rustc_middle::mir::visit::MutVisitor;
use rustc_middle::mir::{
Body, Constant, Local, LocalKind, Location, Operand, Place, Rvalue, StatementKind,
Expand All @@ -29,6 +30,13 @@ use rustc_middle::ty::TyCtxt;

pub struct CopyPropagation;

static NUM_PROPAGATED_LOCAL: Statistic =
Statistic::new(module_path!(), "Number of locals copy propagated");
static NUM_PROPAGATED_CONST: Statistic =
Statistic::new(module_path!(), "Number of constants copy propagated");
static MAX_PROPAGATED: Statistic =
Statistic::new(module_path!(), "Maximum number of copy propagations in a function");
tmiasko marked this conversation as resolved.
Show resolved Hide resolved

impl<'tcx> MirPass<'tcx> for CopyPropagation {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let opts = &tcx.sess.opts.debugging_opts;
Expand All @@ -39,6 +47,8 @@ impl<'tcx> MirPass<'tcx> for CopyPropagation {
return;
}

let mut propagated_local = 0;
let mut propagated_const = 0;
let mut def_use_analysis = DefUseAnalysis::new(body);
loop {
def_use_analysis.analyze(body);
Expand Down Expand Up @@ -139,8 +149,13 @@ impl<'tcx> MirPass<'tcx> for CopyPropagation {
}
}

changed =
action.perform(body, &def_use_analysis, dest_local, location, tcx) || changed;
if action.perform(body, &def_use_analysis, dest_local, location, tcx) {
match action {
Action::PropagateLocalCopy(_) => propagated_local += 1,
Action::PropagateConstant(_) => propagated_const += 1,
}
changed = true;
}
// FIXME(pcwalton): Update the use-def chains to delete the instructions instead of
// regenerating the chains.
break;
Expand All @@ -149,6 +164,10 @@ impl<'tcx> MirPass<'tcx> for CopyPropagation {
break;
}
}

NUM_PROPAGATED_LOCAL.increment(propagated_const);
NUM_PROPAGATED_CONST.increment(propagated_local);
MAX_PROPAGATED.update_max(propagated_local + propagated_const);
}
}

Expand Down Expand Up @@ -193,6 +212,7 @@ fn eliminate_self_assignments(body: &mut Body<'_>, def_use_analysis: &DefUseAnal
changed
}

#[derive(Copy, Clone)]
enum Action<'tcx> {
PropagateLocalCopy(Local),
PropagateConstant(Constant<'tcx>),
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_mir/src/transform/dest_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ use crate::{
util::{dump_mir, PassWhere},
};
use itertools::Itertools;
use rustc_data_structures::statistics::Statistic;
use rustc_data_structures::unify::{InPlaceUnificationTable, UnifyKey};
use rustc_index::{
bit_set::{BitMatrix, BitSet},
Expand All @@ -125,6 +126,13 @@ const MAX_BLOCKS: usize = 250;

pub struct DestinationPropagation;

static NUM_TOO_MANY_LOCALS: Statistic =
Statistic::new(module_path!(), "Number of functions with too many locals to optimize");
static NUM_TOO_MANY_BLOCKS: Statistic =
Statistic::new(module_path!(), "Number of functions with too many blocks to optimize");
static NUM_PROPAGATED: Statistic =
Statistic::new(module_path!(), "Number of destination propagations");

impl<'tcx> MirPass<'tcx> for DestinationPropagation {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
// Only run at mir-opt-level=2 or higher for now (we don't fix up debuginfo and remove
Expand Down Expand Up @@ -164,6 +172,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
"too many candidate locals in {:?} ({}, max is {}), not optimizing",
def_id, relevant, MAX_LOCALS
);
NUM_TOO_MANY_LOCALS.increment(1);
return;
}
if body.basic_blocks().len() > MAX_BLOCKS {
Expand All @@ -173,6 +182,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
body.basic_blocks().len(),
MAX_BLOCKS
);
NUM_TOO_MANY_BLOCKS.increment(1);
return;
}

Expand All @@ -197,6 +207,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
break;
}

NUM_PROPAGATED.increment(1);
replacements.push(candidate);
conflicts.unify(candidate.src, candidate.dest.local);
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_mir/src/transform/inline.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Inlining pass for MIR functions

use rustc_attr as attr;
use rustc_data_structures::statistics::Statistic;
use rustc_hir::def_id::DefId;
use rustc_index::bit_set::BitSet;
use rustc_index::vec::{Idx, IndexVec};
Expand Down Expand Up @@ -28,6 +29,8 @@ const UNKNOWN_SIZE_COST: usize = 10;

pub struct Inline;

static NUM_INLINED: Statistic = Statistic::new(module_path!(), "Number of functions inlined");

#[derive(Copy, Clone, Debug)]
struct CallSite<'tcx> {
callee: DefId,
Expand Down Expand Up @@ -156,6 +159,7 @@ impl Inliner<'tcx> {
continue;
}
debug!("attempting to inline callsite {:?} - success", callsite);
NUM_INLINED.increment(1);

// Add callsites from inlined function
for (bb, bb_data) in caller_body.basic_blocks().iter_enumerated().skip(start) {
Expand Down
Loading