Address review comments

Author: Aaron1011

Cargo.lock

@@ -18,7 +18,6 @@ test = false # we have no unit tests
 doctest = false # and no doc tests
 
 [dependencies]
-measureme = { git = "https://github.com/rust-lang/measureme", rev = "501d6a3c192beee5e633a6c5f79130bedfdadcb5" }
 getrandom = { version = "0.2", features = ["std"] }
 env_logger = "0.8"
 log = "0.4"
@@ -31,6 +30,7 @@ smallvec = "1.4.2"
 # See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`
 # for more information.
 rustc-workspace-hack = "1.0.0"
+measureme = "9.1.2"
 
 # Enable some feature flags that dev-dependencies need but dependencies
 # do not.  This makes `./miri install` after `./miri build` faster.

Cargo.toml

@@ -222,6 +222,10 @@ environment variable:
   times to exclude several variables.  On Windows, the `TERM` environment
   variable is excluded by default.
 * `-Zmiri-ignore-leaks` disables the memory leak checker.
+* `-Zmiri-measureme=<name>` enables `measureme` profiling for the interpreted program.
+   This can be used to find which parts of your program are executing slowly under Miri.
+   The profile is written out to a file with the prefix `<name>`, and can be processed
+   using the tools in the repository https://github.com/rust-lang/measureme.
 * `-Zmiri-seed=<hex>` configures the seed of the RNG that Miri uses to resolve
   non-determinism.  This RNG is used to pick base addresses for allocations.
   When isolation is enabled (the default), this is also used to emulate system
@@ -258,10 +262,6 @@ environment variable:
   this pointer. Note that it is not currently guaranteed that code that works
   with `-Zmiri-track-raw-pointers` also works without
   `-Zmiri-track-raw-pointers`, but for the vast majority of code, this will be the case.
-* `-Zmiri-measureme=<name>` enables `measureme` profiling for the interpreted program.
-   This can be used to find which parts of your program are executing slowly under Miri.
-   The profile is written out to a file with the prefix `<name>`, and can be processed
-   using the tools in the repository https://github.com/rust-lang/measureme
 
 Some native rustc `-Z` flags are also very relevant for Miri:
 

README.md

@@ -54,8 +54,8 @@ pub struct MiriConfig {
     /// Rate of spurious failures for compare_exchange_weak atomic operations,
     /// between 0.0 and 1.0, defaulting to 0.8 (80% chance of failure).
     pub cmpxchg_weak_failure_rate: f64,
-    /// If `Some`, enable the `measureme` profiler, writing results to the specified
-    /// directory.
+    /// If `Some`, enable the `measureme` profiler, writing results to a file
+    /// with the specified prefix.
     pub measureme_out: Option<String>,
 }
 

src/eval.rs

@@ -10,7 +10,6 @@ use std::time::Instant;
 use log::trace;
 use rand::rngs::StdRng;
 use rand::SeedableRng;
-use std::collections::hash_map::Entry;
 use measureme::{Profiler, StringId, EventId, DetachedTiming};
 
 use rustc_data_structures::fx::FxHashMap;
@@ -45,6 +44,9 @@ pub struct FrameData<'tcx> {
     /// we stop unwinding, use the `CatchUnwindData` to handle catching.
     pub catch_unwind: Option<CatchUnwindData<'tcx>>,
 
+    /// If `measureme` profiling is enabled, holds timing information
+    /// for the start of this frame. When we finish executing this frame,
+    /// we use this to register a completed event with `measureme`.
     pub timing: Option<DetachedTiming>,
 }
 
@@ -274,7 +276,11 @@ pub struct Evaluator<'mir, 'tcx> {
     /// Allocations that are considered roots of static memory (that may leak).
     pub(crate) static_roots: Vec<AllocId>,
 
+    /// The `measureme` profiler used to record timing information about
+    /// the emulated program.
     profiler: Option<Profiler>,
+    /// Used with `profiler` to cache the `StringId`s for event names
+    /// uesd with `measureme`.
     string_cache: FxHashMap<String, StringId>,
 }
 
@@ -607,29 +613,28 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
         frame: Frame<'mir, 'tcx, Tag>,
     ) -> InterpResult<'tcx, Frame<'mir, 'tcx, Tag, FrameData<'tcx>>> {
-        let stacked_borrows = ecx.memory.extra.stacked_borrows.as_ref();
-        let call_id = stacked_borrows.map_or(NonZeroU64::new(1).unwrap(), |stacked_borrows| {
-            stacked_borrows.borrow_mut().new_call()
-        });
+        // Start recording our event before doing anything else
         let timing = if let Some(profiler) = ecx.machine.profiler.as_ref() {
             let fn_name = frame.instance.to_string();
             let entry = ecx.machine.string_cache.entry(fn_name.clone());
-            let name = match entry {
-                Entry::Occupied(e) => *e.get(),
-                Entry::Vacant(e) => {
-                    *e.insert(profiler.alloc_string(&*fn_name))
-                }
-            };
+            let name = entry.or_insert_with(|| {
+                profiler.alloc_string(&*fn_name)
+            });
 
             Some(profiler.start_recording_interval_event_detached(
-                name,
-                EventId::from_label(name),
-                ecx.get_active_thread().to_u32()
+                *name,
+                EventId::from_label(*name),
+                ecx.get_active_thread().to_u32(),
             ))
         } else {
             None
         };
 
+        let stacked_borrows = ecx.memory.extra.stacked_borrows.as_ref();
+        let call_id = stacked_borrows.map_or(NonZeroU64::new(1).unwrap(), |stacked_borrows| {
+            stacked_borrows.borrow_mut().new_call()
+        });
+
         let extra = FrameData { call_id, catch_unwind: None, timing };
         Ok(frame.with_extra(extra))
     }