From 1fd2f1687c6483fd3386f838c30332215c3f32b2 Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Mon, 25 Feb 2019 16:44:04 +0100 Subject: [PATCH 01/23] Have all methods of Filter and FilterMap use internal iteration --- src/libcore/iter/adapters/mod.rs | 37 ++++++-------------------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/src/libcore/iter/adapters/mod.rs b/src/libcore/iter/adapters/mod.rs index bca1b76dbb975..d4ad22c16bbfa 100644 --- a/src/libcore/iter/adapters/mod.rs +++ b/src/libcore/iter/adapters/mod.rs @@ -681,12 +681,7 @@ impl Iterator for Filter where P: FnMut(&I::Item) -> bool #[inline] fn next(&mut self) -> Option { - for x in &mut self.iter { - if (self.predicate)(&x) { - return Some(x); - } - } - None + self.try_for_each(Err).err() } #[inline] @@ -707,12 +702,9 @@ impl Iterator for Filter where P: FnMut(&I::Item) -> bool // Using the branchless version will also simplify the LLVM byte code, thus // leaving more budget for LLVM optimizations. #[inline] - fn count(mut self) -> usize { - let mut count = 0; - for x in &mut self.iter { - count += (self.predicate)(&x) as usize; - } - count + fn count(self) -> usize { + let mut predicate = self.predicate; + self.iter.map(|x| predicate(&x) as usize).sum() } #[inline] @@ -746,12 +738,7 @@ impl DoubleEndedIterator for Filter { #[inline] fn next_back(&mut self) -> Option { - for x in self.iter.by_ref().rev() { - if (self.predicate)(&x) { - return Some(x); - } - } - None + self.try_rfold((), |_, x| Err(x)).err() } #[inline] @@ -820,12 +807,7 @@ impl Iterator for FilterMap #[inline] fn next(&mut self) -> Option { - for x in self.iter.by_ref() { - if let Some(y) = (self.f)(x) { - return Some(y); - } - } - None + self.try_for_each(Err).err() } #[inline] @@ -863,12 +845,7 @@ impl DoubleEndedIterator for FilterMap { #[inline] fn next_back(&mut self) -> Option { - for x in self.iter.by_ref().rev() { - if let Some(y) = (self.f)(x) { - return Some(y); - } - } - None + self.try_rfold((), |_, x| Err(x)).err() } #[inline] From d89c2f67e9a8452ebd3bc2150dd05c160e8a360d Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 20 Feb 2019 22:27:00 +0200 Subject: [PATCH 02/23] Use informational target machine for metadata Since there is nothing to optimise there... --- src/librustc_codegen_llvm/back/write.rs | 10 ---------- src/librustc_codegen_llvm/context.rs | 2 +- src/librustc_codegen_llvm/lib.rs | 10 +++++----- src/librustc_codegen_ssa/back/write.rs | 8 +++++++- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index db5430a4219a0..8f991a9ef490b 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -13,10 +13,8 @@ use crate::common; use crate::LlvmCodegenBackend; use rustc_codegen_ssa::back::write::{CodegenContext, ModuleConfig, run_assembler}; use rustc_codegen_ssa::traits::*; -use rustc::hir::def_id::LOCAL_CRATE; use rustc::session::config::{self, OutputType, Passes, Lto}; use rustc::session::Session; -use rustc::ty::TyCtxt; use rustc_codegen_ssa::{ModuleCodegen, CompiledModule}; use rustc::util::common::time_ext; use rustc_fs_util::{path_to_c_string, link_or_copy}; @@ -82,14 +80,6 @@ pub fn write_output_file( } } -pub fn create_target_machine( - tcx: TyCtxt<'_, '_, '_>, - find_features: bool, -) -> &'static mut llvm::TargetMachine { - target_machine_factory(tcx.sess, tcx.backend_optimization_level(LOCAL_CRATE), find_features)() - .unwrap_or_else(|err| llvm_err(tcx.sess.diagnostic(), &err).raise() ) -} - pub fn create_informational_target_machine( sess: &Session, find_features: bool, diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 23e3a8425d370..619304ad9afd0 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -154,7 +154,7 @@ pub unsafe fn create_module( // Ensure the data-layout values hardcoded remain the defaults. if sess.target.target.options.is_builtin { - let tm = crate::back::write::create_target_machine(tcx, false); + let tm = crate::back::write::create_informational_target_machine(&tcx.sess, false); llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, tm); llvm::LLVMRustDisposeTargetMachine(tm); diff --git a/src/librustc_codegen_llvm/lib.rs b/src/librustc_codegen_llvm/lib.rs index 5b8c7461bcb60..38b16078a7d38 100644 --- a/src/librustc_codegen_llvm/lib.rs +++ b/src/librustc_codegen_llvm/lib.rs @@ -24,7 +24,7 @@ #![deny(rust_2018_idioms)] #![allow(explicit_outlives_requirements)] -use back::write::create_target_machine; +use back::write::create_informational_target_machine; use syntax_pos::symbol::Symbol; extern crate flate2; @@ -114,8 +114,9 @@ pub struct LlvmCodegenBackend(()); impl ExtraBackendMethods for LlvmCodegenBackend { fn new_metadata(&self, tcx: TyCtxt<'_, '_, '_>, mod_name: &str) -> ModuleLlvm { - ModuleLlvm::new(tcx, mod_name) + ModuleLlvm::new_metadata(tcx, mod_name) } + fn write_metadata<'b, 'gcx>( &self, tcx: TyCtxt<'b, 'gcx, 'gcx>, @@ -366,15 +367,14 @@ unsafe impl Send for ModuleLlvm { } unsafe impl Sync for ModuleLlvm { } impl ModuleLlvm { - fn new(tcx: TyCtxt<'_, '_, '_>, mod_name: &str) -> Self { + fn new_metadata(tcx: TyCtxt<'_, '_, '_>, mod_name: &str) -> Self { unsafe { let llcx = llvm::LLVMRustContextCreate(tcx.sess.fewer_names()); let llmod_raw = context::create_module(tcx, llcx, mod_name) as *const _; - ModuleLlvm { llmod_raw, llcx, - tm: create_target_machine(tcx, false), + tm: create_informational_target_machine(&tcx.sess, false), } } } diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 908ee95efcba3..2bade64ddbd34 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -1022,7 +1022,13 @@ fn start_executing_work( None }; - let ol = tcx.backend_optimization_level(LOCAL_CRATE); + let ol = if tcx.sess.opts.debugging_opts.no_codegen + || !tcx.sess.opts.output_types.should_codegen() { + // If we know that we won’t be doing codegen, create target machines without optimisation. + config::OptLevel::No + } else { + tcx.backend_optimization_level(LOCAL_CRATE) + }; let cgcx = CodegenContext:: { backend: backend.clone(), crate_types: sess.crate_types.borrow().clone(), From 88847718f0cca57124cbd1bce12fc938c5bde088 Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Wed, 27 Feb 2019 11:44:30 +0100 Subject: [PATCH 03/23] Add relevant benchmarks --- src/libcore/benches/iter.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/libcore/benches/iter.rs b/src/libcore/benches/iter.rs index fe852e42b5cd3..5cc2bce846d87 100644 --- a/src/libcore/benches/iter.rs +++ b/src/libcore/benches/iter.rs @@ -306,3 +306,31 @@ fn bench_skip_then_zip(b: &mut Bencher) { assert_eq!(s, 2009900); }); } + +#[bench] +fn bench_filter_count(b: &mut Bencher) { + b.iter(|| { + (0i64..1000000).map(black_box).filter(|x| x % 3 == 0).count() + }) +} + +#[bench] +fn bench_filter_ref_count(b: &mut Bencher) { + b.iter(|| { + (0i64..1000000).map(black_box).by_ref().filter(|x| x % 3 == 0).count() + }) +} + +#[bench] +fn bench_filter_chain_count(b: &mut Bencher) { + b.iter(|| { + (0i64..1000000).chain(0..1000000).map(black_box).filter(|x| x % 3 == 0).count() + }) +} + +#[bench] +fn bench_filter_chain_ref_count(b: &mut Bencher) { + b.iter(|| { + (0i64..1000000).chain(0..1000000).map(black_box).by_ref().filter(|x| x % 3 == 0).count() + }) +} \ No newline at end of file From ec2e4ba919a65a972019925b5b33d0f309f467fc Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Wed, 27 Feb 2019 11:46:37 +0100 Subject: [PATCH 04/23] Improve existing benchmarks to prevent extreme optimizations --- src/libcore/benches/iter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/benches/iter.rs b/src/libcore/benches/iter.rs index 5cc2bce846d87..4f3d6f379022b 100644 --- a/src/libcore/benches/iter.rs +++ b/src/libcore/benches/iter.rs @@ -185,13 +185,13 @@ bench_sums! { bench_sums! { bench_filter_sum, bench_filter_ref_sum, - (0i64..1000000).filter(|x| x % 2 == 0) + (0i64..1000000).filter(|x| x % 3 == 0) } bench_sums! { bench_filter_chain_sum, bench_filter_chain_ref_sum, - (0i64..1000000).chain(0..1000000).filter(|x| x % 2 == 0) + (0i64..1000000).chain(0..1000000).filter(|x| x % 3 == 0) } bench_sums! { From 88bd624a88692dd4bbda5f3f324f0035dc041534 Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Wed, 27 Feb 2019 13:22:18 +0100 Subject: [PATCH 05/23] Add trailing newline --- src/libcore/benches/iter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/benches/iter.rs b/src/libcore/benches/iter.rs index 4f3d6f379022b..1dd2bd3ee78aa 100644 --- a/src/libcore/benches/iter.rs +++ b/src/libcore/benches/iter.rs @@ -333,4 +333,4 @@ fn bench_filter_chain_ref_count(b: &mut Bencher) { b.iter(|| { (0i64..1000000).chain(0..1000000).map(black_box).by_ref().filter(|x| x % 3 == 0).count() }) -} \ No newline at end of file +} From 4e7d4c7778a00371ba707fb8f9022bcbb443bb29 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 27 Feb 2019 15:32:32 +0100 Subject: [PATCH 06/23] ManuallyDrop != MaybeUninit --- src/libcore/mem.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index f41d293e80ad3..6b1b91d00faa7 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -900,10 +900,16 @@ pub fn discriminant(v: &T) -> Discriminant { } } +// FIXME: Reference `MaybeUninit` from these docs, once that is stable. /// A wrapper to inhibit compiler from automatically calling `T`’s destructor. /// /// This wrapper is 0-cost. /// +/// `ManuallyDrop` is subject to the same layout optimizations as `T`. +/// As a consequence, it has *no effect* on the assumptions that the compiler makes +/// about all values being initialized at their type. In particular, initializing +/// a `ManuallyDrop<&T>` with [`mem::zeroed`] is undefined behavior. +/// /// # Examples /// /// This wrapper helps with explicitly documenting the drop order dependencies between fields of @@ -935,6 +941,8 @@ pub fn discriminant(v: &T) -> Discriminant { /// } /// } /// ``` +/// +/// [`mem::zeroed']: fn.zeroed.html #[stable(feature = "manually_drop", since = "1.20.0")] #[lang = "manually_drop"] #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] From b70a9532a9d3e2ae4b9bf7bc4a4a0bfc7dbe134b Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Wed, 20 Feb 2019 15:11:22 +0100 Subject: [PATCH 07/23] Replace `s` with `self` in docs for str methods taking self. --- src/libcore/str/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 8b51d8465141a..53334adadb856 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -3965,7 +3965,7 @@ impl str { me.make_ascii_lowercase() } - /// Return an iterator that escapes each char in `s` with [`char::escape_debug`]. + /// Return an iterator that escapes each char in `self` with [`char::escape_debug`]. /// /// Note: only extended grapheme codepoints that begin the string will be /// escaped. @@ -4013,7 +4013,7 @@ impl str { } } - /// Return an iterator that escapes each char in `s` with [`char::escape_default`]. + /// Return an iterator that escapes each char in `self` with [`char::escape_default`]. /// /// [`char::escape_default`]: ../std/primitive.char.html#method.escape_default /// @@ -4051,7 +4051,7 @@ impl str { EscapeDefault { inner: self.chars().flat_map(CharEscapeDefault) } } - /// Return an iterator that escapes each char in `s` with [`char::escape_unicode`]. + /// Return an iterator that escapes each char in `self` with [`char::escape_unicode`]. /// /// [`char::escape_unicode`]: ../std/primitive.char.html#method.escape_unicode /// From f92c20426ea5f5ddcc6dcf73dbe0739d3d76b614 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 27 Feb 2019 18:58:19 +0100 Subject: [PATCH 08/23] improve readability --- src/libcore/mem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 6b1b91d00faa7..f78213ba9f672 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -908,7 +908,7 @@ pub fn discriminant(v: &T) -> Discriminant { /// `ManuallyDrop` is subject to the same layout optimizations as `T`. /// As a consequence, it has *no effect* on the assumptions that the compiler makes /// about all values being initialized at their type. In particular, initializing -/// a `ManuallyDrop<&T>` with [`mem::zeroed`] is undefined behavior. +/// a `ManuallyDrop<&mut T>` with [`mem::zeroed`] is undefined behavior. /// /// # Examples /// From a998b1f425bc13d891ed5b28f1b708970f9db4b4 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Wed, 27 Feb 2019 10:56:58 -0500 Subject: [PATCH 09/23] allow specifying attributes for tool lints --- src/librustc/lint/mod.rs | 20 +++++++++++++------ .../ui-fulldeps/auxiliary/lint_tool_test.rs | 6 +++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index dd003e44bea09..496ff568b31b4 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -132,14 +132,22 @@ macro_rules! declare_lint { #[macro_export] macro_rules! declare_tool_lint { - ($vis: vis $tool: ident ::$NAME: ident, $Level: ident, $desc: expr) => ( - declare_tool_lint!{$vis $tool::$NAME, $Level, $desc, false} + ( + $(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level: ident, $desc: expr + ) => ( + declare_tool_lint!{$(#[$attr])* $vis $tool::$NAME, $Level, $desc, false} ); - ($vis: vis $tool: ident ::$NAME: ident, $Level: ident, $desc: expr, - report_in_external_macro: $rep: expr) => ( - declare_tool_lint!{$vis $tool::$NAME, $Level, $desc, $rep} + ( + $(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level:ident, $desc:expr, + report_in_external_macro: $rep:expr + ) => ( + declare_tool_lint!{$(#[$attr])* $vis $tool::$NAME, $Level, $desc, $rep} ); - ($vis: vis $tool: ident ::$NAME: ident, $Level: ident, $desc: expr, $external: expr) => ( + ( + $(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level:ident, $desc:expr, + $external:expr + ) => ( + $(#[$attr])* $vis static $NAME: &$crate::lint::Lint = &$crate::lint::Lint { name: &concat!(stringify!($tool), "::", stringify!($NAME)), default_level: $crate::lint::$Level, diff --git a/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs b/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs index 1a9bd9e66dbac..d25a5ea374627 100644 --- a/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs +++ b/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs @@ -13,7 +13,11 @@ use rustc::lint::{EarlyContext, LintContext, LintPass, EarlyLintPass, use rustc_plugin::Registry; use syntax::ast; declare_tool_lint!(pub clippy::TEST_LINT, Warn, "Warn about stuff"); -declare_tool_lint!(pub clippy::TEST_GROUP, Warn, "Warn about other stuff"); +declare_tool_lint!( + /// Some docs + pub clippy::TEST_GROUP, + Warn, "Warn about other stuff" +); struct Pass; From 0c1a38ce3b281ce23b9ba84312f58ba6ac82af57 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 28 Feb 2019 09:24:35 +0100 Subject: [PATCH 10/23] Update src/libcore/mem.rs Co-Authored-By: RalfJung --- src/libcore/mem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index f78213ba9f672..94f342e7e8efc 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -942,7 +942,7 @@ pub fn discriminant(v: &T) -> Discriminant { /// } /// ``` /// -/// [`mem::zeroed']: fn.zeroed.html +/// [`mem::zeroed`]: fn.zeroed.html #[stable(feature = "manually_drop", since = "1.20.0")] #[lang = "manually_drop"] #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] From 96be181c7ed004b2892addc22c285b321e029fd9 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 28 Feb 2019 16:34:03 -0500 Subject: [PATCH 11/23] Fixed a syntax error in the pin docs --- src/libcore/pin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/pin.rs b/src/libcore/pin.rs index f9f20dcea9e2e..835357b4e017c 100644 --- a/src/libcore/pin.rs +++ b/src/libcore/pin.rs @@ -215,7 +215,7 @@ //! had a method `fn get_pin_mut(self: Pin<&mut Self>) -> Pin<&mut T>`. //! Then we could do the following: //! ```compile_fail -//! fn exploit_ref_cell(rc: Pin<&mut RefCell) { +//! fn exploit_ref_cell(rc: Pin<&mut RefCell>) { //! { let p = rc.as_mut().get_pin_mut(); } // Here we get pinned access to the `T`. //! let rc_shr: &RefCell = rc.into_ref().get_ref(); //! let b = rc_shr.borrow_mut(); From f50cf9425e86ce434cfbc68f4d628a6e0e52b2a8 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sun, 10 Feb 2019 17:23:00 -0500 Subject: [PATCH 12/23] Wrap the self-profiler in an `Arc>` This will allow us to send it across threads and measure things like LLVM time. --- Cargo.lock | 1 + src/librustc/session/mod.rs | 46 ++++++++++++-------------- src/librustc_codegen_ssa/Cargo.toml | 1 + src/librustc_codegen_ssa/back/write.rs | 24 ++++++++++++++ src/librustc_codegen_ssa/lib.rs | 3 ++ src/librustc_driver/driver.rs | 8 ----- src/librustc_driver/lib.rs | 9 +++++ 7 files changed, 60 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 381322bc421d4..2c2c4e566f1e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2624,6 +2624,7 @@ dependencies = [ "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "memmap 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "rustc 0.0.0", "rustc-demangle 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_allocator 0.0.0", diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 5b9b70edc6809..c86e1ed4e8846 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -43,7 +43,9 @@ use std::fmt; use std::io::Write; use std::path::PathBuf; use std::time::Duration; -use std::sync::mpsc; +use std::sync::{Arc, mpsc}; + +use parking_lot::Mutex as PlMutex; mod code_stats; pub mod config; @@ -126,11 +128,8 @@ pub struct Session { /// Used by `-Z profile-queries` in `util::common`. pub profile_channel: Lock>>, - /// Used by `-Z self-profile`. - pub self_profiling_active: bool, - - /// Used by `-Z self-profile`. - pub self_profiling: Lock, + /// Used by -Z self-profile + pub self_profiling: Option>>, /// Some measurements that are being gathered during compilation. pub perf_stats: PerfStats, @@ -833,27 +832,23 @@ impl Session { #[inline(never)] #[cold] fn profiler_active ()>(&self, f: F) { - let mut profiler = self.self_profiling.borrow_mut(); - f(&mut profiler); + match &self.self_profiling { + None => bug!("profiler_active() called but there was no profiler active"), + Some(profiler) => { + let mut p = profiler.lock(); + + f(&mut p); + } + } } #[inline(always)] pub fn profiler ()>(&self, f: F) { - if unlikely!(self.self_profiling_active) { + if unlikely!(self.self_profiling.is_some()) { self.profiler_active(f) } } - pub fn print_profiler_results(&self) { - let mut profiler = self.self_profiling.borrow_mut(); - profiler.print_results(&self.opts); - } - - pub fn save_json_results(&self) { - let profiler = self.self_profiling.borrow(); - profiler.save_results(&self.opts); - } - pub fn print_perf_stats(&self) { println!( "Total time spent computing symbol hashes: {}", @@ -1135,6 +1130,13 @@ pub fn build_session_( source_map: Lrc, driver_lint_caps: FxHashMap, ) -> Session { + let self_profiling_active = sopts.debugging_opts.self_profile || + sopts.debugging_opts.profile_json; + + let self_profiler = + if self_profiling_active { Some(Arc::new(PlMutex::new(SelfProfiler::new()))) } + else { None }; + let host_triple = TargetTriple::from_triple(config::host_triple()); let host = Target::search(&host_triple).unwrap_or_else(|e| span_diagnostic @@ -1184,9 +1186,6 @@ pub fn build_session_( CguReuseTracker::new_disabled() }; - let self_profiling_active = sopts.debugging_opts.self_profile || - sopts.debugging_opts.profile_json; - let sess = Session { target: target_cfg, host, @@ -1215,8 +1214,7 @@ pub fn build_session_( imported_macro_spans: OneThread::new(RefCell::new(FxHashMap::default())), incr_comp_session: OneThread::new(RefCell::new(IncrCompSession::NotInitialized)), cgu_reuse_tracker, - self_profiling_active, - self_profiling: Lock::new(SelfProfiler::new()), + self_profiling: self_profiler, profile_channel: Lock::new(None), perf_stats: PerfStats { symbol_hash_time: Lock::new(Duration::from_secs(0)), diff --git a/src/librustc_codegen_ssa/Cargo.toml b/src/librustc_codegen_ssa/Cargo.toml index 0aba43580f1f6..4702e34aa19e7 100644 --- a/src/librustc_codegen_ssa/Cargo.toml +++ b/src/librustc_codegen_ssa/Cargo.toml @@ -19,6 +19,7 @@ memmap = "0.6" log = "0.4.5" libc = "0.2.44" jobserver = "0.1.11" +parking_lot = "0.7" serialize = { path = "../libserialize" } syntax = { path = "../libsyntax" } diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 908ee95efcba3..4bccc2a6d1f7b 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -19,6 +19,7 @@ use rustc::util::time_graph::{self, TimeGraph, Timeline}; use rustc::hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc::ty::TyCtxt; use rustc::util::common::{time_depth, set_time_depth, print_time_passes_entry}; +use rustc::util::profiling::SelfProfiler; use rustc_fs_util::link_or_copy; use rustc_data_structures::svh::Svh; use rustc_errors::{Handler, Level, DiagnosticBuilder, FatalError, DiagnosticId}; @@ -29,6 +30,7 @@ use syntax::ext::hygiene::Mark; use syntax_pos::MultiSpan; use syntax_pos::symbol::Symbol; use jobserver::{Client, Acquired}; +use parking_lot::Mutex as PlMutex; use std::any::Any; use std::fs; @@ -201,6 +203,7 @@ pub struct CodegenContext { // Resources needed when running LTO pub backend: B, pub time_passes: bool, + pub profiler: Option>>, pub lto: Lto, pub no_landing_pads: bool, pub save_temps: bool, @@ -254,6 +257,26 @@ impl CodegenContext { ModuleKind::Allocator => &self.allocator_module_config, } } + + #[inline(never)] + #[cold] + fn profiler_active ()>(&self, f: F) { + match &self.profiler { + None => bug!("profiler_active() called but there was no profiler active"), + Some(profiler) => { + let mut p = profiler.lock(); + + f(&mut p); + } + } + } + + #[inline(always)] + pub fn profile ()>(&self, f: F) { + if unlikely!(self.profiler.is_some()) { + self.profiler_active(f) + } + } } fn generate_lto_work( @@ -1033,6 +1056,7 @@ fn start_executing_work( save_temps: sess.opts.cg.save_temps, opts: Arc::new(sess.opts.clone()), time_passes: sess.time_passes(), + profiler: sess.self_profiling.clone(), exported_symbols, plugin_passes: sess.plugin_llvm_passes.borrow().clone(), remark: sess.opts.cg.remark.clone(), diff --git a/src/librustc_codegen_ssa/lib.rs b/src/librustc_codegen_ssa/lib.rs index 92fa2766f8727..9685fda426296 100644 --- a/src/librustc_codegen_ssa/lib.rs +++ b/src/librustc_codegen_ssa/lib.rs @@ -2,9 +2,11 @@ #![feature(box_patterns)] #![feature(box_syntax)] +#![feature(core_intrinsics)] #![feature(custom_attribute)] #![feature(libc)] #![feature(rustc_diagnostic_macros)] +#![feature(stmt_expr_attributes)] #![feature(in_band_lifetimes)] #![feature(nll)] #![allow(unused_attributes)] @@ -20,6 +22,7 @@ #[macro_use] extern crate log; #[macro_use] extern crate rustc; +#[macro_use] extern crate rustc_data_structures; #[macro_use] extern crate syntax; use std::path::PathBuf; diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index f87a809e6c6eb..c9f07c9888bda 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -345,14 +345,6 @@ pub fn compile_input( sess.print_perf_stats(); } - if sess.opts.debugging_opts.self_profile { - sess.print_profiler_results(); - } - - if sess.opts.debugging_opts.profile_json { - sess.save_json_results(); - } - controller_entry_point!( compilation_done, sess, diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 52dbb618d0d11..9546498cf939a 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -276,6 +276,15 @@ fn run_compiler_with_pool<'a>( &control) }; + + if sess.opts.debugging_opts.self_profile { + sess.profiler(|p| p.print_results(&sess.opts)); + } + + if sess.opts.debugging_opts.profile_json { + sess.profiler(|p| p.save_results(&sess.opts)); + } + (result, Some(sess)) } From 979ece9ed714a0d22ad50b8026dabc72951e842a Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 11 Feb 2019 18:11:43 -0500 Subject: [PATCH 13/23] Remove profiler output and replace with a raw event dump Related to #58372 --- src/librustc/session/config.rs | 4 +- src/librustc/session/mod.rs | 5 +- src/librustc/util/profiling.rs | 584 +++++++++++++++------------------ src/librustc_driver/lib.rs | 7 +- 4 files changed, 271 insertions(+), 329 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 99eee4b559a5c..774ab0333db54 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1405,9 +1405,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, crate_attr: Vec = (Vec::new(), parse_string_push, [TRACKED], "inject the given attribute in the crate"), self_profile: bool = (false, parse_bool, [UNTRACKED], - "run the self profiler"), - profile_json: bool = (false, parse_bool, [UNTRACKED], - "output a json file with profiler results"), + "run the self profiler and output the raw event data"), emit_stack_sizes: bool = (false, parse_bool, [UNTRACKED], "emits a section containing stack size metadata"), plt: Option = (None, parse_opt_bool, [TRACKED], diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index c86e1ed4e8846..286a96d8f767a 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -1130,11 +1130,8 @@ pub fn build_session_( source_map: Lrc, driver_lint_caps: FxHashMap, ) -> Session { - let self_profiling_active = sopts.debugging_opts.self_profile || - sopts.debugging_opts.profile_json; - let self_profiler = - if self_profiling_active { Some(Arc::new(PlMutex::new(SelfProfiler::new()))) } + if sopts.debugging_opts.self_profile { Some(Arc::new(PlMutex::new(SelfProfiler::new()))) } else { None }; let host_triple = TargetTriple::from_triple(config::host_triple()); diff --git a/src/librustc/util/profiling.rs b/src/librustc/util/profiling.rs index c90bd12a3100f..bd40566065b6b 100644 --- a/src/librustc/util/profiling.rs +++ b/src/librustc/util/profiling.rs @@ -1,10 +1,12 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::HashMap; use std::fs; -use std::io::{self, Write}; +use std::io::{BufWriter, Write}; +use std::mem; +use std::process; use std::thread::ThreadId; use std::time::Instant; -use crate::session::config::{Options, OptLevel}; +use crate::session::config::Options; #[derive(Clone, Copy, Debug, PartialEq, Eq, Ord, PartialOrd)] pub enum ProfileCategory { @@ -23,140 +25,39 @@ pub enum ProfilerEvent { QueryEnd { query_name: &'static str, category: ProfileCategory, time: Instant }, GenericActivityStart { category: ProfileCategory, time: Instant }, GenericActivityEnd { category: ProfileCategory, time: Instant }, - QueryCacheHit { query_name: &'static str, category: ProfileCategory }, - QueryCount { query_name: &'static str, category: ProfileCategory, count: usize }, IncrementalLoadResultStart { query_name: &'static str, time: Instant }, IncrementalLoadResultEnd { query_name: &'static str, time: Instant }, + QueryCacheHit { query_name: &'static str, category: ProfileCategory, time: Instant }, + QueryCount { query_name: &'static str, category: ProfileCategory, count: usize, time: Instant }, QueryBlockedStart { query_name: &'static str, category: ProfileCategory, time: Instant }, QueryBlockedEnd { query_name: &'static str, category: ProfileCategory, time: Instant }, } impl ProfilerEvent { - fn is_start_event(&self) -> bool { + fn timestamp(&self) -> Instant { use self::ProfilerEvent::*; match self { - QueryStart { .. } | - GenericActivityStart { .. } | - IncrementalLoadResultStart { .. } | - QueryBlockedStart { .. } => true, - - QueryEnd { .. } | - GenericActivityEnd { .. } | - QueryCacheHit { .. } | - QueryCount { .. } | - IncrementalLoadResultEnd { .. } | - QueryBlockedEnd { .. } => false, + QueryStart { time, .. } | + QueryEnd { time, .. } | + GenericActivityStart { time, .. } | + GenericActivityEnd { time, .. } | + QueryCacheHit { time, .. } | + QueryCount { time, .. } | + IncrementalLoadResultStart { time, .. } | + IncrementalLoadResultEnd { time, .. } | + QueryBlockedStart { time, .. } | + QueryBlockedEnd { time, .. } => *time } } } -pub struct SelfProfiler { - events: HashMap>, -} - -struct CategoryResultData { - query_times: BTreeMap<&'static str, u64>, - query_cache_stats: BTreeMap<&'static str, (u64, u64)>, //(hits, total) -} - -impl CategoryResultData { - fn new() -> CategoryResultData { - CategoryResultData { - query_times: BTreeMap::new(), - query_cache_stats: BTreeMap::new(), - } - } - - fn total_time(&self) -> u64 { - self.query_times.iter().map(|(_, time)| time).sum() - } - - fn total_cache_data(&self) -> (u64, u64) { - let (mut hits, mut total) = (0, 0); - - for (_, (h, t)) in &self.query_cache_stats { - hits += h; - total += t; - } - - (hits, total) - } -} - -impl Default for CategoryResultData { - fn default() -> CategoryResultData { - CategoryResultData::new() - } -} - -struct CalculatedResults { - categories: BTreeMap, - crate_name: Option, - optimization_level: OptLevel, - incremental: bool, - verbose: bool, -} - -impl CalculatedResults { - fn new() -> CalculatedResults { - CalculatedResults { - categories: BTreeMap::new(), - crate_name: None, - optimization_level: OptLevel::No, - incremental: false, - verbose: false, - } - } - - fn consolidate(mut cr1: CalculatedResults, cr2: CalculatedResults) -> CalculatedResults { - for (category, data) in cr2.categories { - let cr1_data = cr1.categories.entry(category).or_default(); - - for (query, time) in data.query_times { - *cr1_data.query_times.entry(query).or_default() += time; - } - - for (query, (hits, total)) in data.query_cache_stats { - let (h, t) = cr1_data.query_cache_stats.entry(query).or_insert((0, 0)); - *h += hits; - *t += total; - } - } - - cr1 - } - - fn total_time(&self) -> u64 { - self.categories.iter().map(|(_, data)| data.total_time()).sum() - } - - fn with_options(mut self, opts: &Options) -> CalculatedResults { - self.crate_name = opts.crate_name.clone(); - self.optimization_level = opts.optimize; - self.incremental = opts.incremental.is_some(); - self.verbose = opts.debugging_opts.verbose; - - self - } +fn thread_id_to_u64(tid: ThreadId) -> u64 { + unsafe { mem::transmute::(tid) } } -fn time_between_ns(start: Instant, end: Instant) -> u64 { - if start < end { - let time = end - start; - (time.as_secs() * 1_000_000_000) + (time.subsec_nanos() as u64) - } else { - debug!("time_between_ns: ignorning instance of end < start"); - 0 - } -} - -fn calculate_percent(numerator: u64, denominator: u64) -> f32 { - if denominator > 0 { - ((numerator as f32) / (denominator as f32)) * 100.0 - } else { - 0.0 - } +pub struct SelfProfiler { + events: HashMap>, } impl SelfProfiler { @@ -197,6 +98,7 @@ impl SelfProfiler { query_name, category, count, + time: Instant::now(), }) } @@ -205,6 +107,7 @@ impl SelfProfiler { self.record(ProfilerEvent::QueryCacheHit { query_name, category, + time: Instant::now(), }) } @@ -268,208 +171,257 @@ impl SelfProfiler { events.push(event); } - fn calculate_thread_results(events: &Vec) -> CalculatedResults { + pub fn dump_raw_events(&self, opts: &Options) { use self::ProfilerEvent::*; - assert!( - events.last().map(|e| !e.is_start_event()).unwrap_or(true), - "there was an event running when calculate_reslts() was called" - ); - - let mut results = CalculatedResults::new(); - - //(event, child time to subtract) - let mut query_stack = Vec::new(); - - for event in events { - match event { - QueryStart { .. } | GenericActivityStart { .. } => { - query_stack.push((event, 0)); - }, - QueryEnd { query_name, category, time: end_time } => { - let previous_query = query_stack.pop(); - if let Some((QueryStart { - query_name: p_query_name, - time: start_time, - category: _ }, child_time_to_subtract)) = previous_query { - assert_eq!( - p_query_name, - query_name, - "Saw a query end but the previous query wasn't the corresponding start" - ); - - let time_ns = time_between_ns(*start_time, *end_time); - let self_time_ns = time_ns - child_time_to_subtract; - let result_data = results.categories.entry(*category).or_default(); - - *result_data.query_times.entry(query_name).or_default() += self_time_ns; - - if let Some((_, child_time_to_subtract)) = query_stack.last_mut() { - *child_time_to_subtract += time_ns; - } - } else { - bug!("Saw a query end but the previous event wasn't a query start"); - } + //find the earliest Instant to use as t=0 + //when serializing the events, we'll calculate a Duration + //using (instant - min_instant) + let min_instant = + self.events + .iter() + .map(|(_, values)| values[0].timestamp()) + .min() + .unwrap(); + + let pid = process::id(); + + let filename = + format!("{}.profile_events.json", opts.crate_name.clone().unwrap_or_default()); + + let mut file = BufWriter::new(fs::File::create(filename).unwrap()); + + let threads: Vec<_> = + self.events + .keys() + .into_iter() + .map(|tid| format!("{}", thread_id_to_u64(*tid))) + .collect(); + + write!(file, + "{{\ + \"processes\": {{\ + \"{}\": {{\ + \"threads\": [{}],\ + \"crate_name\": \"{}\",\ + \"opt_level\": \"{:?}\",\ + \"incremental\": {}\ + }}\ + }},\ + \"events\": [\ + ", + pid, + threads.join(","), + opts.crate_name.clone().unwrap_or_default(), + opts.optimize, + if opts.incremental.is_some() { "true" } else { "false" }, + ).unwrap(); + + let mut is_first = true; + for (thread_id, events) in &self.events { + let thread_id = thread_id_to_u64(*thread_id); + + for event in events { + if is_first { + is_first = false; + } else { + writeln!(file, ",").unwrap(); } - GenericActivityEnd { category, time: end_time } => { - let previous_event = query_stack.pop(); - if let Some((GenericActivityStart { - category: previous_category, - time: start_time }, child_time_to_subtract)) = previous_event { - assert_eq!( - previous_category, - category, - "Saw an end but the previous event wasn't the corresponding start" - ); - - let time_ns = time_between_ns(*start_time, *end_time); - let self_time_ns = time_ns - child_time_to_subtract; - let result_data = results.categories.entry(*category).or_default(); - - *result_data.query_times - .entry("{time spent not running queries}") - .or_default() += self_time_ns; - - if let Some((_, child_time_to_subtract)) = query_stack.last_mut() { - *child_time_to_subtract += time_ns; - } - } else { - bug!("Saw an activity end but the previous event wasn't an activity start"); - } - }, - QueryCacheHit { category, query_name } => { - let result_data = results.categories.entry(*category).or_default(); - - let (hits, total) = - result_data.query_cache_stats.entry(query_name).or_insert((0, 0)); - *hits += 1; - *total += 1; - }, - QueryCount { category, query_name, count } => { - let result_data = results.categories.entry(*category).or_default(); - - let (_, totals) = - result_data.query_cache_stats.entry(query_name).or_insert((0, 0)); - *totals += *count as u64; - }, - //we don't summarize incremental load result events in the simple output mode - IncrementalLoadResultStart { .. } | IncrementalLoadResultEnd { .. } => { }, - //we don't summarize parallel query blocking in the simple output mode - QueryBlockedStart { .. } | QueryBlockedEnd { .. } => { }, - } - } - //normalize the times to ms - for (_, data) in &mut results.categories { - for (_, time) in &mut data.query_times { - *time = *time / 1_000_000; - } - } - - results - } - - fn get_results(&self, opts: &Options) -> CalculatedResults { - self.events - .iter() - .map(|(_, r)| SelfProfiler::calculate_thread_results(r)) - .fold(CalculatedResults::new(), CalculatedResults::consolidate) - .with_options(opts) - } - - pub fn print_results(&mut self, opts: &Options) { - self.end_activity(ProfileCategory::Other); - - let results = self.get_results(opts); - - let total_time = results.total_time() as f32; - - let out = io::stderr(); - let mut lock = out.lock(); - - let crate_name = results.crate_name.map(|n| format!(" for {}", n)).unwrap_or_default(); - - writeln!(lock, "Self profiling results{}:", crate_name).unwrap(); - writeln!(lock).unwrap(); - - writeln!(lock, "| Phase | Time (ms) \ - | Time (%) | Queries | Hits (%)") - .unwrap(); - writeln!(lock, "| ----------------------------------------- | -------------- \ - | -------- | -------------- | --------") - .unwrap(); - - let mut categories: Vec<_> = results.categories.iter().collect(); - categories.sort_by_cached_key(|(_, d)| d.total_time()); - - for (category, data) in categories.iter().rev() { - let (category_hits, category_total) = data.total_cache_data(); - let category_hit_percent = calculate_percent(category_hits, category_total); - - writeln!( - lock, - "| {0: <41} | {1: >14} | {2: >8.2} | {3: >14} | {4: >8}", - format!("{:?}", category), - data.total_time(), - ((data.total_time() as f32) / total_time) * 100.0, - category_total, - format!("{:.2}", category_hit_percent), - ).unwrap(); - - //in verbose mode, show individual query data - if results.verbose { - //don't show queries that took less than 1ms - let mut times: Vec<_> = data.query_times.iter().filter(|(_, t)| **t > 0).collect(); - times.sort_by(|(_, time1), (_, time2)| time2.cmp(time1)); - - for (query, time) in times { - let (hits, total) = data.query_cache_stats.get(query).unwrap_or(&(0, 0)); - let hit_percent = calculate_percent(*hits, *total); - - writeln!( - lock, - "| - {0: <39} | {1: >14} | {2: >8.2} | {3: >14} | {4: >8}", - query, - time, - ((*time as f32) / total_time) * 100.0, - total, - format!("{:.2}", hit_percent), - ).unwrap(); + let (secs, nanos) = { + let duration = event.timestamp() - min_instant; + (duration.as_secs(), duration.subsec_nanos()) + }; + + match event { + QueryStart { query_name, category, time: _ } => + write!(file, + "{{ \ + \"QueryStart\": {{ \ + \"query_name\": \"{}\",\ + \"category\": \"{:?}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + category, + secs, + nanos, + thread_id, + ).unwrap(), + QueryEnd { query_name, category, time: _ } => + write!(file, + "{{\ + \"QueryEnd\": {{\ + \"query_name\": \"{}\",\ + \"category\": \"{:?}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + category, + secs, + nanos, + thread_id, + ).unwrap(), + GenericActivityStart { category, time: _ } => + write!(file, + "{{ + \"GenericActivityStart\": {{\ + \"category\": \"{:?}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + category, + secs, + nanos, + thread_id, + ).unwrap(), + GenericActivityEnd { category, time: _ } => + write!(file, + "{{\ + \"GenericActivityEnd\": {{\ + \"category\": \"{:?}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + category, + secs, + nanos, + thread_id, + ).unwrap(), + QueryCacheHit { query_name, category, time: _ } => + write!(file, + "{{\ + \"QueryCacheHit\": {{\ + \"query_name\": \"{}\",\ + \"category\": \"{:?}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + category, + secs, + nanos, + thread_id, + ).unwrap(), + QueryCount { query_name, category, count, time: _ } => + write!(file, + "{{\ + \"QueryCount\": {{\ + \"query_name\": \"{}\",\ + \"category\": \"{:?}\",\ + \"count\": {},\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + category, + count, + secs, + nanos, + thread_id, + ).unwrap(), + IncrementalLoadResultStart { query_name, time: _ } => + write!(file, + "{{\ + \"IncrementalLoadResultStart\": {{\ + \"query_name\": \"{}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + secs, + nanos, + thread_id, + ).unwrap(), + IncrementalLoadResultEnd { query_name, time: _ } => + write!(file, + "{{\ + \"IncrementalLoadResultEnd\": {{\ + \"query_name\": \"{}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + secs, + nanos, + thread_id, + ).unwrap(), + QueryBlockedStart { query_name, category, time: _ } => + write!(file, + "{{\ + \"QueryBlockedStart\": {{\ + \"query_name\": \"{}\",\ + \"category\": \"{:?}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + category, + secs, + nanos, + thread_id, + ).unwrap(), + QueryBlockedEnd { query_name, category, time: _ } => + write!(file, + "{{\ + \"QueryBlockedEnd\": {{\ + \"query_name\": \"{}\",\ + \"category\": \"{:?}\",\ + \"time\": {{\ + \"secs\": {},\ + \"nanos\": {}\ + }},\ + \"thread_id\": {}\ + }}\ + }}", + query_name, + category, + secs, + nanos, + thread_id, + ).unwrap() } } } - writeln!(lock).unwrap(); - writeln!(lock, "Optimization level: {:?}", opts.optimize).unwrap(); - writeln!(lock, "Incremental: {}", if results.incremental { "on" } else { "off" }).unwrap(); - } - - pub fn save_results(&self, opts: &Options) { - let results = self.get_results(opts); - - let compilation_options = - format!("{{ \"optimization_level\": \"{:?}\", \"incremental\": {} }}", - results.optimization_level, - if results.incremental { "true" } else { "false" }); - - let mut category_data = String::new(); - - for (category, data) in &results.categories { - let (hits, total) = data.total_cache_data(); - let hit_percent = calculate_percent(hits, total); - - category_data.push_str(&format!("{{ \"category\": \"{:?}\", \"time_ms\": {}, \ - \"query_count\": {}, \"query_hits\": {} }}", - category, - data.total_time(), - total, - format!("{:.2}", hit_percent))); - } - - let json = format!("{{ \"category_data\": {}, \"compilation_options\": {} }}", - category_data, - compilation_options); - - fs::write("self_profiler_results.json", json).unwrap(); + write!(file, "] }}").unwrap(); } } diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 9546498cf939a..cc1b8916c1074 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -276,13 +276,8 @@ fn run_compiler_with_pool<'a>( &control) }; - if sess.opts.debugging_opts.self_profile { - sess.profiler(|p| p.print_results(&sess.opts)); - } - - if sess.opts.debugging_opts.profile_json { - sess.profiler(|p| p.save_results(&sess.opts)); + sess.profiler(|p| p.dump_raw_events(&sess.opts)); } (result, Some(sess)) From 2fb8d60a584bed5d0b893bfd3cf091cbebf158e2 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 13 Feb 2019 05:33:51 -0500 Subject: [PATCH 14/23] Reduce the size of events by using a u64 instead of Instant Part of #58372 --- src/librustc/util/profiling.rs | 74 +++++++++++++++++----------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/src/librustc/util/profiling.rs b/src/librustc/util/profiling.rs index bd40566065b6b..c052f00cd18e3 100644 --- a/src/librustc/util/profiling.rs +++ b/src/librustc/util/profiling.rs @@ -4,7 +4,7 @@ use std::io::{BufWriter, Write}; use std::mem; use std::process; use std::thread::ThreadId; -use std::time::Instant; +use std::time::{Duration, Instant, SystemTime}; use crate::session::config::Options; @@ -21,20 +21,20 @@ pub enum ProfileCategory { #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ProfilerEvent { - QueryStart { query_name: &'static str, category: ProfileCategory, time: Instant }, - QueryEnd { query_name: &'static str, category: ProfileCategory, time: Instant }, - GenericActivityStart { category: ProfileCategory, time: Instant }, - GenericActivityEnd { category: ProfileCategory, time: Instant }, - IncrementalLoadResultStart { query_name: &'static str, time: Instant }, - IncrementalLoadResultEnd { query_name: &'static str, time: Instant }, - QueryCacheHit { query_name: &'static str, category: ProfileCategory, time: Instant }, - QueryCount { query_name: &'static str, category: ProfileCategory, count: usize, time: Instant }, - QueryBlockedStart { query_name: &'static str, category: ProfileCategory, time: Instant }, - QueryBlockedEnd { query_name: &'static str, category: ProfileCategory, time: Instant }, + QueryStart { query_name: &'static str, category: ProfileCategory, time: u64 }, + QueryEnd { query_name: &'static str, category: ProfileCategory, time: u64 }, + GenericActivityStart { category: ProfileCategory, time: u64 }, + GenericActivityEnd { category: ProfileCategory, time: u64 }, + IncrementalLoadResultStart { query_name: &'static str, time: u64 }, + IncrementalLoadResultEnd { query_name: &'static str, time: u64 }, + QueryCacheHit { query_name: &'static str, category: ProfileCategory, time: u64 }, + QueryCount { query_name: &'static str, category: ProfileCategory, count: usize, time: u64 }, + QueryBlockedStart { query_name: &'static str, category: ProfileCategory, time: u64 }, + QueryBlockedEnd { query_name: &'static str, category: ProfileCategory, time: u64 }, } impl ProfilerEvent { - fn timestamp(&self) -> Instant { + fn timestamp(&self) -> u64 { use self::ProfilerEvent::*; match self { @@ -58,16 +58,18 @@ fn thread_id_to_u64(tid: ThreadId) -> u64 { pub struct SelfProfiler { events: HashMap>, + start_time: SystemTime, + start_instant: Instant, } impl SelfProfiler { pub fn new() -> SelfProfiler { - let mut profiler = SelfProfiler { + let profiler = SelfProfiler { events: HashMap::new(), + start_time: SystemTime::now(), + start_instant: Instant::now(), }; - profiler.start_activity(ProfileCategory::Other); - profiler } @@ -75,7 +77,7 @@ impl SelfProfiler { pub fn start_activity(&mut self, category: ProfileCategory) { self.record(ProfilerEvent::GenericActivityStart { category, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -83,7 +85,7 @@ impl SelfProfiler { pub fn end_activity(&mut self, category: ProfileCategory) { self.record(ProfilerEvent::GenericActivityEnd { category, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -98,7 +100,7 @@ impl SelfProfiler { query_name, category, count, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -107,7 +109,7 @@ impl SelfProfiler { self.record(ProfilerEvent::QueryCacheHit { query_name, category, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -116,7 +118,7 @@ impl SelfProfiler { self.record(ProfilerEvent::QueryStart { query_name, category, - time: Instant::now(), + time: self.get_time_from_start(), }); } @@ -125,7 +127,7 @@ impl SelfProfiler { self.record(ProfilerEvent::QueryEnd { query_name, category, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -133,7 +135,7 @@ impl SelfProfiler { pub fn incremental_load_result_start(&mut self, query_name: &'static str) { self.record(ProfilerEvent::IncrementalLoadResultStart { query_name, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -141,7 +143,7 @@ impl SelfProfiler { pub fn incremental_load_result_end(&mut self, query_name: &'static str) { self.record(ProfilerEvent::IncrementalLoadResultEnd { query_name, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -150,7 +152,7 @@ impl SelfProfiler { self.record(ProfilerEvent::QueryBlockedStart { query_name, category, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -159,7 +161,7 @@ impl SelfProfiler { self.record(ProfilerEvent::QueryBlockedEnd { query_name, category, - time: Instant::now(), + time: self.get_time_from_start(), }) } @@ -171,19 +173,15 @@ impl SelfProfiler { events.push(event); } + #[inline] + fn get_time_from_start(&self) -> u64 { + let duration = Instant::now() - self.start_instant; + duration.as_nanos() as u64 + } + pub fn dump_raw_events(&self, opts: &Options) { use self::ProfilerEvent::*; - //find the earliest Instant to use as t=0 - //when serializing the events, we'll calculate a Duration - //using (instant - min_instant) - let min_instant = - self.events - .iter() - .map(|(_, values)| values[0].timestamp()) - .min() - .unwrap(); - let pid = process::id(); let filename = @@ -229,8 +227,10 @@ impl SelfProfiler { } let (secs, nanos) = { - let duration = event.timestamp() - min_instant; - (duration.as_secs(), duration.subsec_nanos()) + let time = self.start_time + Duration::from_nanos(event.timestamp()); + let time_since_unix = + time.duration_since(SystemTime::UNIX_EPOCH).unwrap_or_default(); + (time_since_unix.as_secs(), time_since_unix.subsec_nanos()) }; match event { From d1ac8774e94b5c457ba07a2f4279a4b25705f67b Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 25 Feb 2019 18:46:53 -0500 Subject: [PATCH 15/23] Use FxHashMap --- src/librustc/util/profiling.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc/util/profiling.rs b/src/librustc/util/profiling.rs index c052f00cd18e3..c134d48f987be 100644 --- a/src/librustc/util/profiling.rs +++ b/src/librustc/util/profiling.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::fs; use std::io::{BufWriter, Write}; use std::mem; @@ -8,6 +7,8 @@ use std::time::{Duration, Instant, SystemTime}; use crate::session::config::Options; +use rustc_data_structures::fx::FxHashMap; + #[derive(Clone, Copy, Debug, PartialEq, Eq, Ord, PartialOrd)] pub enum ProfileCategory { Parsing, @@ -57,7 +58,7 @@ fn thread_id_to_u64(tid: ThreadId) -> u64 { } pub struct SelfProfiler { - events: HashMap>, + events: FxHashMap>, start_time: SystemTime, start_instant: Instant, } @@ -65,7 +66,7 @@ pub struct SelfProfiler { impl SelfProfiler { pub fn new() -> SelfProfiler { let profiler = SelfProfiler { - events: HashMap::new(), + events: Default::default(), start_time: SystemTime::now(), start_instant: Instant::now(), }; From 3391f6ce8083ea860009caa99b415b402feeddd1 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 1 Mar 2019 10:14:09 +0100 Subject: [PATCH 16/23] tidy: deny(rust_2018_idioms) --- src/tools/tidy/src/deps.rs | 12 ++++++------ src/tools/tidy/src/features.rs | 2 +- src/tools/tidy/src/lib.rs | 3 ++- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 94dd5478e5297..0169725fa2968 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -49,13 +49,13 @@ const EXCEPTIONS: &[&str] = &[ ]; /// Which crates to check against the whitelist? -const WHITELIST_CRATES: &[CrateVersion] = &[ +const WHITELIST_CRATES: &[CrateVersion<'_>] = &[ CrateVersion("rustc", "0.0.0"), CrateVersion("rustc_codegen_llvm", "0.0.0"), ]; /// Whitelist of crates rustc is allowed to depend on. Avoid adding to the list if possible. -const WHITELIST: &[Crate] = &[ +const WHITELIST: &[Crate<'_>] = &[ Crate("adler32"), Crate("aho-corasick"), Crate("arrayvec"), @@ -183,7 +183,7 @@ struct Crate<'a>(&'a str); // (name) #[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Debug, Hash)] struct CrateVersion<'a>(&'a str, &'a str); // (name, version) -impl<'a> Crate<'a> { +impl Crate<'_> { pub fn id_str(&self) -> String { format!("{} ", self.0) } @@ -330,10 +330,10 @@ fn get_deps(path: &Path, cargo: &Path) -> Resolve { /// Checks the dependencies of the given crate from the given cargo metadata to see if they are on /// the whitelist. Returns a list of illegal dependencies. -fn check_crate_whitelist<'a, 'b>( - whitelist: &'a HashSet, +fn check_crate_whitelist<'a>( + whitelist: &'a HashSet>, resolve: &'a Resolve, - visited: &'b mut BTreeSet>, + visited: &mut BTreeSet>, krate: CrateVersion<'a>, must_be_on_whitelist: bool, ) -> BTreeSet> { diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 7126c0c2f6ecf..1eab217027c8a 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -22,7 +22,7 @@ pub enum Status { } impl fmt::Display for Status { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let as_str = match *self { Status::Stable => "stable", Status::Unstable => "unstable", diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 022c53f909c75..c4a1246ffdf55 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -3,7 +3,8 @@ //! This library contains the tidy lints and exposes it //! to be used by tools. -extern crate serde; +#![deny(rust_2018_idioms)] + extern crate serde_json; #[macro_use] extern crate serde_derive; From 805145f2f96545600f7fec0a9a7b9edfb7bd9c38 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Fri, 1 Mar 2019 10:41:25 +0100 Subject: [PATCH 17/23] Revert "Auto merge of #58597 - pietroalbini:appveyor-gce, r=alexcrichton" This reverts commit fd42f24b0129b32d66f174510518c083cdcec3eb, reversing changes made to 0e25a6829c66302dc06c351bb494774e3d075f77. --- appveyor.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 3a0cb8b4fceea..d70ad54b1c812 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -5,11 +5,6 @@ environment: # server goes down presumably. See #43333 for more info CARGO_HTTP_CHECK_REVOKE: false - # Execute the builds on GCE instead of Hyper-V. Those builders have a 3-4 - # minute startup overhead, but AppVeyor support recommended this as a - # possible solution for #58160 (spurious 259 exit codes) - appveyor_build_worker_cloud: gce - matrix: # 32/64 bit MSVC tests - MSYS_BITS: 64 From 260b0917b074b00960672a5308edee5c023e44f4 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 1 Mar 2019 11:15:22 +0100 Subject: [PATCH 18/23] tools/rustbook: deny(rust_2018_idioms) --- src/tools/rustbook/src/main.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tools/rustbook/src/main.rs b/src/tools/rustbook/src/main.rs index 5a6246347cc03..cfc1bc6d414e4 100644 --- a/src/tools/rustbook/src/main.rs +++ b/src/tools/rustbook/src/main.rs @@ -1,4 +1,5 @@ -// +#![deny(rust_2018_idioms)] + use clap::{crate_version}; use std::env; @@ -68,7 +69,7 @@ fn main() { } // Build command implementation -pub fn build_1(args: &ArgMatches) -> Result1<()> { +pub fn build_1(args: &ArgMatches<'_>) -> Result1<()> { let book_dir = get_book_dir(args); let mut book = MDBook1::load(&book_dir)?; @@ -85,7 +86,7 @@ pub fn build_1(args: &ArgMatches) -> Result1<()> { } // Build command implementation -pub fn build_2(args: &ArgMatches) -> Result2<()> { +pub fn build_2(args: &ArgMatches<'_>) -> Result2<()> { let book_dir = get_book_dir(args); let mut book = MDBook2::load(&book_dir)?; @@ -101,7 +102,7 @@ pub fn build_2(args: &ArgMatches) -> Result2<()> { Ok(()) } -fn get_book_dir(args: &ArgMatches) -> PathBuf { +fn get_book_dir(args: &ArgMatches<'_>) -> PathBuf { if let Some(dir) = args.value_of("dir") { // Check if path is relative from current dir, or absolute... let p = Path::new(dir); From c259489209449a49d136dab4f09f7fcadd8f078c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 1 Mar 2019 11:23:25 +0100 Subject: [PATCH 19/23] tools/remote-test-{client,server}: deny(rust_2018_idioms) --- src/tools/remote-test-client/src/main.rs | 4 +++- src/tools/remote-test-server/src/main.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tools/remote-test-client/src/main.rs b/src/tools/remote-test-client/src/main.rs index cb9dac27ce491..f42de44176787 100644 --- a/src/tools/remote-test-client/src/main.rs +++ b/src/tools/remote-test-client/src/main.rs @@ -1,3 +1,5 @@ +#![deny(rust_2018_idioms)] + /// This is a small client program intended to pair with `remote-test-server` in /// this repository. This client connects to the server over TCP and is used to /// push artifacts and run tests on the server instead of locally. @@ -15,7 +17,7 @@ use std::process::{Command, Stdio}; use std::thread; use std::time::Duration; -const REMOTE_ADDR_ENV: &'static str = "TEST_DEVICE_ADDR"; +const REMOTE_ADDR_ENV: &str = "TEST_DEVICE_ADDR"; macro_rules! t { ($e:expr) => (match $e { diff --git a/src/tools/remote-test-server/src/main.rs b/src/tools/remote-test-server/src/main.rs index 750eea3a28aef..e1270489d315f 100644 --- a/src/tools/remote-test-server/src/main.rs +++ b/src/tools/remote-test-server/src/main.rs @@ -1,3 +1,5 @@ +#![deny(rust_2018_idioms)] + /// This is a small server which is intended to run inside of an emulator or /// on a remote test device. This server pairs with the `remote-test-client` /// program in this repository. The `remote-test-client` connects to this @@ -120,7 +122,7 @@ struct RemoveOnDrop<'a> { inner: &'a Path, } -impl<'a> Drop for RemoveOnDrop<'a> { +impl Drop for RemoveOnDrop<'_> { fn drop(&mut self) { t!(fs::remove_dir_all(self.inner)); } From 670a4d65d577cfc59eefe8eb0b73d5b027ea0038 Mon Sep 17 00:00:00 2001 From: Jens Hausdorf Date: Fri, 1 Mar 2019 13:19:00 +0100 Subject: [PATCH 20/23] Fix typo in Vec#resize_with documentation --- src/liballoc/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 229dafc5fdc3a..947ce354ae711 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -1260,7 +1260,7 @@ impl Vec { /// This method uses a closure to create new values on every push. If /// you'd rather [`Clone`] a given value, use [`resize`]. If you want /// to use the [`Default`] trait to generate values, you can pass - /// [`Default::default()`] as the second argument.. + /// [`Default::default()`] as the second argument. /// /// # Examples /// From c3aab1448070e7c039c204889fe59a1d00456db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Miku=C5=82a?= Date: Fri, 1 Mar 2019 15:06:14 +0100 Subject: [PATCH 21/23] Forbid duplicating Cargo as a dependency --- src/tools/tidy/src/deps.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 94dd5478e5297..fbc4730bc1d6e 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -378,7 +378,7 @@ fn check_crate_duplicate(resolve: &Resolve, bad: &mut bool) { // to accidentally sneak into our dependency graph, in order to ensure we keep our CI times // under control. - // "cargo", // FIXME(#53005) + "cargo", "rustc-ap-syntax", ]; let mut name_to_id: HashMap<_, Vec<_>> = HashMap::new(); From 04d0a8cb83e713f71848c4afeba72d7e776acc6a Mon Sep 17 00:00:00 2001 From: Dan Robertson Date: Sat, 2 Mar 2019 03:14:29 +0000 Subject: [PATCH 22/23] Fix C-variadic function printing There is no longer a need to append the string `", ..."` to a functions args as `...` is parsed as an argument and will appear in the functions arguments. --- src/libsyntax/print/pprust.rs | 3 --- src/test/pretty/fn-variadic.rs | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 src/test/pretty/fn-variadic.rs diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 942bd96939173..49e3fad4af0ff 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -2814,9 +2814,6 @@ impl<'a> State<'a> { -> io::Result<()> { self.popen()?; self.commasep(Inconsistent, &decl.inputs, |s, arg| s.print_arg(arg, false))?; - if decl.c_variadic { - self.s.word(", ...")?; - } self.pclose()?; self.print_fn_output(decl) diff --git a/src/test/pretty/fn-variadic.rs b/src/test/pretty/fn-variadic.rs new file mode 100644 index 0000000000000..d3b193a3e1814 --- /dev/null +++ b/src/test/pretty/fn-variadic.rs @@ -0,0 +1,15 @@ +// Check that `fn foo(x: i32, ...)` does not print as `fn foo(x: i32, ..., ...)`. +// See issue #58853. +// +// pp-exact +#![feature(c_variadic)] + +extern "C" { + pub fn foo(x: i32, ...); +} + +pub unsafe extern "C" fn bar(_: i32, mut ap: ...) -> usize { + ap.arg::() +} + +fn main() { } From 72f0ca51ef3d946a0a43eadad7b938001186572b Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Sat, 2 Mar 2019 15:16:36 +0000 Subject: [PATCH 23/23] Nit --- src/test/pretty/fn-variadic.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/pretty/fn-variadic.rs b/src/test/pretty/fn-variadic.rs index d3b193a3e1814..59e477cfa8ecb 100644 --- a/src/test/pretty/fn-variadic.rs +++ b/src/test/pretty/fn-variadic.rs @@ -1,6 +1,6 @@ // Check that `fn foo(x: i32, ...)` does not print as `fn foo(x: i32, ..., ...)`. // See issue #58853. -// + // pp-exact #![feature(c_variadic)] @@ -12,4 +12,4 @@ pub unsafe extern "C" fn bar(_: i32, mut ap: ...) -> usize { ap.arg::() } -fn main() { } +fn main() {}