diff --git a/tests/c/goto_loop.c b/tests/c/goto_loop.c index ba87f3d0f..46c7215b6 100644 --- a/tests/c/goto_loop.c +++ b/tests/c/goto_loop.c @@ -1,4 +1,5 @@ // Run-time: +// env-var: YKD_SERIALISE_COMPILATION=1 // stdout: 1000 // stderr: diff --git a/tests/c/indirect_branch.c b/tests/c/indirect_branch.c index 372f2ba0e..d475dd648 100644 --- a/tests/c/indirect_branch.c +++ b/tests/c/indirect_branch.c @@ -3,6 +3,7 @@ // Run-time: // env-var: YKD_LOG_IR=jit-pre-opt // env-var: YKD_LOG=3 +// env-var: YKD_SERIALISE_COMPILATION=1 // stderr: // FIXME: match some IR/output // ... diff --git a/tests/c/jit_disabled.c b/tests/c/jit_disabled.c index 9274a2615..71edfa6a6 100644 --- a/tests/c/jit_disabled.c +++ b/tests/c/jit_disabled.c @@ -1,6 +1,7 @@ // Run-time: // env-var: YK_JITC=none // env-var: YKD_LOG=4 +// env-var: YKD_SERIALISE_COMPILATION=1 // stderr: // 4 // 3 diff --git a/tests/c/phi1.c b/tests/c/phi1.c index 66035fa6d..d3ace2e7d 100644 --- a/tests/c/phi1.c +++ b/tests/c/phi1.c @@ -4,6 +4,7 @@ // env-var: YKD_LOG_IR=aot,jit-pre-opt // env-var: YKD_LOG=4 // env-var: YKD_LOG_STATS=/dev/null +// env-var: YKD_SERIALISE_COMPILATION=1 // stderr: // yk-tracing: start-tracing // i=4, val=1 diff --git a/tests/c/phi3.c b/tests/c/phi3.c index 4a6adec7c..0fe7a2fc3 100644 --- a/tests/c/phi3.c +++ b/tests/c/phi3.c @@ -4,6 +4,7 @@ // env-var: YKD_LOG_IR=aot,jit-pre-opt // env-var: YKD_LOG=4 // env-var: YKD_LOG_STATS=/dev/null +// env-var: YKD_SERIALISE_COMPILATION=1 // stderr: // yk-tracing: start-tracing // i=4, val=3 diff --git a/tests/c/ptradd.c b/tests/c/ptradd.c index 66ee3f11b..426a0e63c 100644 --- a/tests/c/ptradd.c +++ b/tests/c/ptradd.c @@ -4,6 +4,7 @@ // env-var: YKD_LOG_IR=aot,jit-pre-opt // env-var: YKD_LOG=4 // env-var: YKD_LOG_STATS=/dev/null +// env-var: YKD_SERIALISE_COMPILATION=1 // stderr: // yk-tracing: start-tracing // i=4, val=3, p=4 diff --git a/tests/c/simple_interp_loop1.c b/tests/c/simple_interp_loop1.c index 69140bb35..739ece5a1 100644 --- a/tests/c/simple_interp_loop1.c +++ b/tests/c/simple_interp_loop1.c @@ -1,5 +1,6 @@ // Run-time: // env-var: YKD_LOG=4 +// env-var: YKD_SERIALISE_COMPILATION=1 // env-var: YKD_LOG_STATS=/dev/null // stderr: // yk-tracing: start-tracing diff --git a/tests/c/simple_interp_loop2.c b/tests/c/simple_interp_loop2.c index 7ff434f64..c38e08945 100644 --- a/tests/c/simple_interp_loop2.c +++ b/tests/c/simple_interp_loop2.c @@ -1,6 +1,7 @@ // Run-time: // env-var: YKD_LOG=4 // env-var: YKD_LOG_STATS=/dev/null +// env-var: YKD_SERIALISE_COMPILATION=1 // stderr: // yk-tracing: start-tracing // pc=0, mem=4 diff --git a/tests/c/stats4.c b/tests/c/stats4.c index 5536398bf..301b73748 100644 --- a/tests/c/stats4.c +++ b/tests/c/stats4.c @@ -1,5 +1,6 @@ // Run-time: // env-var: YKD_LOG_STATS=- +// env-var: YKD_SERIALISE_COMPILATION=1 // stderr: // { // ... diff --git a/tests/lua/unrolling.lua b/tests/lua/unrolling.lua new file mode 100644 index 000000000..0b9ca724b --- /dev/null +++ b/tests/lua/unrolling.lua @@ -0,0 +1,77 @@ +for i = 0, 5 do + if i > 2 then + for j = 0, 2 do + io.stderr:write("j ", tostring(j), "\n") + end + end + io.stderr:write("i ", tostring(i), "\n") +end +io.stderr:write("exit\n") + +-- Run-time: +-- env-var: YK_HOT_THRESHOLD=3 +-- env-var: YK_SIDETRACE_THRESHOLD=2 +-- env-var: YKD_LOG=4 +-- env-var: YKD_LOG_IR=debugstrs +-- env-var: YKD_SERIALISE_COMPILATION=1 +-- stderr: +-- i 0 +-- i 1 +-- i 2 +-- yk-tracing: start-tracing: unrolling.lua:2: GTI +-- j 0 +-- yk-warning: tracing-aborted: tracing unrolled a loop: unrolling.lua:4: GETTABUP +-- j 1 +-- j 2 +-- i 3 +-- j 0 +-- j 1 +-- yk-tracing: start-tracing: unrolling.lua:4: GETTABUP +-- j 2 +-- i 4 +-- yk-tracing: stop-tracing: unrolling.lua:4: GETTABUP +-- --- Begin debugstrs: unrolling.lua:4: GETTABUP --- +-- ; { +-- ; "trid": "1", +-- ; "start": { +-- ; "kind": "ControlPoint" +-- ; }, +-- ; "end": { +-- ; "kind": "Loop" +-- ; } +-- ; } +-- unrolling.lua:4: GETTABUP +-- unrolling.lua:4: GETFIELD +-- unrolling.lua:4: SELF +-- unrolling.lua:4: LOADK +-- unrolling.lua:4: GETTABUP +-- unrolling.lua:4: MOVE +-- unrolling.lua:4: CALL +-- unrolling.lua:4: LOADK +-- unrolling.lua:4: CALL +-- unrolling.lua:3: FORLOOP +-- unrolling.lua:7: GETTABUP +-- unrolling.lua:7: GETFIELD +-- unrolling.lua:7: SELF +-- unrolling.lua:7: LOADK +-- unrolling.lua:7: GETTABUP +-- unrolling.lua:7: MOVE +-- unrolling.lua:7: CALL +-- unrolling.lua:7: LOADK +-- unrolling.lua:7: CALL +-- unrolling.lua:1: FORLOOP +-- unrolling.lua:2: GTI +-- unrolling.lua:3: LOADI +-- unrolling.lua:3: LOADI +-- unrolling.lua:3: LOADI +-- unrolling.lua:3: FORPREP +-- --- End debugstrs --- +-- j 0 +-- yk-execution: enter-jit-code: unrolling.lua:4: GETTABUP +-- j 1 +-- yk-execution: deoptimise ... +-- yk-execution: enter-jit-code: unrolling.lua:4: GETTABUP +-- j 2 +-- i 5 +-- yk-execution: deoptimise ... +-- exit diff --git a/ykrt/src/compile/j2/mod.rs b/ykrt/src/compile/j2/mod.rs index 9a5a94cf5..b8d460611 100644 --- a/ykrt/src/compile/j2/mod.rs +++ b/ykrt/src/compile/j2/mod.rs @@ -25,13 +25,11 @@ mod x64; use crate::{ compile::{ - CompilationError, CompiledTrace, Compiler, GuardId, j2::codebuf::CodeBufInProgress, - jitc_yk::AOT_MOD, + CompilationError, CompiledTrace, Compiler, Trace, TraceEnd, TraceStart, + j2::codebuf::CodeBufInProgress, jitc_yk::AOT_MOD, }, - location::HotLocation, log::{IRPhase, should_log_ir}, - mt::{MT, TraceId}, - trace::AOTTraceIterator, + mt::MT, }; use libc::{MAP_ANON, MAP_FAILED, MAP_PRIVATE, PROT_READ, PROT_WRITE, mmap, munmap}; use parking_lot::Mutex; @@ -225,19 +223,28 @@ impl J2 { } impl Compiler for J2 { - fn root_compile( + fn compile( self: Arc, mt: Arc, - ta_iter: Box, - trid: TraceId, - hl: Arc>, - promotions: Box<[u8]>, - debug_strs: Vec, - coupler: Option>, + trace: Trace, ) -> Result, CompilationError> { - let kind = match coupler { - Some(tgt_ctr) => aot_to_hir::BuildKind::Coupler { tgt_ctr }, - None => aot_to_hir::BuildKind::Loop, + let (hl, bkind) = match (trace.trace_start, &trace.trace_end) { + (TraceStart::ControlPoint { hl }, TraceEnd::Loop) => (hl, aot_to_hir::BuildKind::Loop), + (TraceStart::ControlPoint { hl }, TraceEnd::Coupler(coupler_tid)) => ( + hl, + aot_to_hir::BuildKind::Coupler { + tgt_ctr: mt.compiled_trace(*coupler_tid), + }, + ), + (TraceStart::Guard { .. }, TraceEnd::Loop) => unreachable!(), + (TraceStart::Guard { parent_ctr, gid }, TraceEnd::Coupler(coupler_tid)) => ( + parent_ctr.hl().upgrade().unwrap(), + aot_to_hir::BuildKind::Side { + src_ctr: parent_ctr, + src_gid: gid, + tgt_ctr: mt.compiled_trace(*coupler_tid), + }, + ), }; #[cfg(target_arch = "x86_64")] @@ -248,61 +255,11 @@ impl Compiler for J2 { &self, &AOT_MOD, Arc::clone(&hl), - ta_iter, - trid, - kind, - promotions, - debug_strs, - ) - .build()?; - - let log = should_log_ir(IRPhase::Asm); - - #[cfg(target_arch = "x86_64")] - let minlen = x64::x64hir_to_asm::X64HirToAsm::codebuf_minlen(&hm); - let buf = self.mmap_codebufinprogress(minlen); - #[cfg(target_arch = "x86_64")] - let be = x64::x64hir_to_asm::X64HirToAsm::new(&hm, buf, log); - - let ct = hir_to_asm::HirToAsm::new(&hm, hl, be, log).build(mt.clone())?; - - // Register JITted code (if required). - mt.trace_profiler().register_ctr(&ct).map_err(|e| { - CompilationError::General(format!("failed to register jitted code with profiler: {e}")) - })?; - - Ok(ct) - } - - fn sidetrace_compile( - self: Arc, - mt: Arc, - ta_iter: Box, - trid: TraceId, - src_ctr: Arc, - src_gid: GuardId, - tgt_ctr: Arc, - hl: Arc>, - promotions: Box<[u8]>, - debug_strs: Vec, - ) -> Result, CompilationError> { - #[cfg(target_arch = "x86_64")] - type AotToHir = aot_to_hir::AotToHir; - - let hm = AotToHir::new( - &mt, - &self, - &AOT_MOD, - Arc::clone(&hl), - ta_iter, - trid, - aot_to_hir::BuildKind::Side { - src_ctr, - src_gid, - tgt_ctr, - }, - promotions, - debug_strs, + trace.ta_iter, + trace.ctrid, + bkind, + trace.promotions, + trace.debug_strs, ) .build()?; diff --git a/ykrt/src/compile/mod.rs b/ykrt/src/compile/mod.rs index 602ecbe97..91ebec247 100644 --- a/ykrt/src/compile/mod.rs +++ b/ykrt/src/compile/mod.rs @@ -43,30 +43,11 @@ pub(crate) enum CompilationError { /// The trait that every JIT compiler backend must implement. pub(crate) trait Compiler: Send + Sync { - /// Compile a mapped root trace into machine code. - fn root_compile( + /// Compile the [Trace] `trace`. + fn compile( self: Arc, mt: Arc, - aottrace_iter: Box, - ctrid: TraceId, - hl: Arc>, - promotions: Box<[u8]>, - debug_strs: Vec, - connector_ctr: Option>, - ) -> Result, CompilationError>; - - /// Compile a guard trace into machine code. - fn sidetrace_compile( - self: Arc, - mt: Arc, - aottrace_iter: Box, - ctrid: TraceId, - parent_ctr: Arc, - gid: GuardId, - target_ctr: Arc, - hl: Arc>, - promotions: Box<[u8]>, - debug_strs: Vec, + trace: Trace, ) -> Result, CompilationError>; } @@ -79,6 +60,40 @@ pub(crate) fn default_compiler() -> Result, Box> { } } +/// A recorded (but not yet compiled) trace. +pub(crate) struct Trace { + pub(crate) trace_start: TraceStart, + pub(crate) trace_end: TraceEnd, + /// The [TraceId] this trace should have when compiled. + pub(crate) ctrid: TraceId, + pub(crate) ta_iter: Box, + pub(crate) promotions: Box<[u8]>, + pub(crate) debug_strs: Vec, +} + +/// How a recorded trace started. +#[derive(Clone)] +pub(crate) enum TraceStart { + /// This trace started at a control point. + ControlPoint { hl: Arc> }, + /// This trace started at guard [TraceStart::Guard::gid] in [TraceStart::Guard::parent_ctr]. + Guard { + parent_ctr: Arc, + gid: GuardId, + }, +} + +/// How a recorded trace ended. +#[derive(Clone)] +pub(crate) enum TraceEnd { + /// A [TraceStart::ControlPoint] trace looped. Note: by definition a [TraceStart::Guard] can + /// not end in [TraceEnd::Loop]. + Loop, + /// A trace ended at another (possibly compiled, possibly tracing, possibly compiling) trace + /// that has/will have the tid [TraceId]. + Coupler(TraceId), +} + pub(crate) trait CompiledTrace: fmt::Debug + Send + Sync { /// Return this trace's [TraceId]. fn ctrid(&self) -> TraceId; diff --git a/ykrt/src/job_queue.rs b/ykrt/src/job_queue.rs index cedfa1f1e..30d101a66 100644 --- a/ykrt/src/job_queue.rs +++ b/ykrt/src/job_queue.rs @@ -18,25 +18,25 @@ use std::{ pub(crate) struct Job { /// The function we expect to run for this job when it is ready. main: Box, - /// If `Some`, wait until [TraceID] has compiled before running this job. - connector_tid: Option, - /// If `connector_tid` failed to compile, this function will be run. - connector_failed: Box, + /// If `Some`, wait until [TraceId] has compiled before running this job. + coupler_tid: Option, + /// If [Self::coupler_tid] failed to compile, this function will be run. + coupler_failed: Box, } impl Job { - /// Create a new job with a `main` method. If `connector_tid` is `Some`, `main` will only be - /// run when `connector_tid` has compiled. If `connector_tid` fails to compile, then - /// `connector_failed` will be run (and `main` will not be run). + /// Create a new job with a `main` method. If `coupler_tid` is `Some`, `main` will only be + /// run when `coupler_tid` has compiled. If `coupler_tid` fails to compile, then + /// `coupler_failed` will be run (and `main` will not be run). pub(crate) fn new( main: Box, - connector_tid: Option, - connector_failed: Box, + coupler_tid: Option, + coupler_failed: Box, ) -> Self { Self { main, - connector_tid, - connector_failed, + coupler_tid, + coupler_failed, } } } @@ -51,8 +51,8 @@ pub(crate) struct JobQueue { /// percolate the error upwards, making it more likely that the main thread exits with an /// error. In other words, this [Vec] makes it harder for errors to be missed. worker_threads: Mutex>>, - /// The ordered queue of compilation worker functions, each a pair `(Option, - /// job)`. Before `job` is run, `connector_tid`, if it is `Some`, must be present in + /// The ordered queue of compilation worker functions, each a pair `(Option, + /// job)`. Before `job` is run, `coupler_tid`, if it is `Some`, must be present in /// [MT::compiled_traces]. queue: Arc<(Condvar, Mutex>)>, } @@ -120,7 +120,7 @@ impl JobQueue { // spin up a new thread for each compilation. This is only acceptable because a) // `SERIALISE_COMPILATION` is an internal yk testing feature b) when we use it we're // checking correctness, not performance. - if let Some(tid) = job.connector_tid + if let Some(tid) = job.coupler_tid && !mt.compiled_traces.lock().contains_key(&tid) { self.queue.1.lock().push_back(job); @@ -131,7 +131,7 @@ impl JobQueue { let mut lk = self.queue.1.lock(); let cnd = { let ct_lk = mt.compiled_traces.lock(); - lk.iter().position(|x| match &x.connector_tid { + lk.iter().position(|x| match &x.coupler_tid { Some(x) => ct_lk.contains_key(x), None => true, }) @@ -171,12 +171,12 @@ impl JobQueue { // point trying to do further work, even if there is work in the queue. while let Some(mt_st) = mt_wk.upgrade() { // Search through the queue looking for the first job we can compile (i.e. - // there is no connector trace ID, or the connector trade ID has been + // there is no coupler trace ID, or the coupler trade ID has been // compiled). self_cl.idle_worker_threads.fetch_sub(1, Ordering::Relaxed); let cnd = { let ct_lk = mt_st.compiled_traces.lock(); - lk.iter().position(|x| match &x.connector_tid { + lk.iter().position(|x| match &x.coupler_tid { Some(x) => ct_lk.contains_key(x), None => true, }) @@ -212,7 +212,7 @@ impl JobQueue { let cnt = { let lk = self.queue.1.lock(); lk.iter() - .filter(|job| match &job.connector_tid { + .filter(|job| match &job.coupler_tid { Some(x) => *x == trid, None => false, }) @@ -232,7 +232,7 @@ impl JobQueue { let mut i = 0; let mut lk = self.queue.1.lock(); while i < lk.len() { - if lk[i].connector_tid == Some(trid) { + if lk[i].coupler_tid == Some(trid) { removed.push(lk.remove(i).unwrap()); } else { i += 1; @@ -240,7 +240,7 @@ impl JobQueue { } drop(lk); for x in removed { - (x.connector_failed)(); + (x.coupler_failed)(); } } } diff --git a/ykrt/src/location.rs b/ykrt/src/location.rs index 96fd73641..945a00db3 100644 --- a/ykrt/src/location.rs +++ b/ykrt/src/location.rs @@ -332,6 +332,28 @@ impl std::fmt::Debug for HotLocationKind { } } +/// Track [HotLocation]s seen while tracing to determine if unrolling has occurred. +pub(super) struct SeenHotLocations { + seen: Vec>>, +} + +impl SeenHotLocations { + /// Create a new [SeenHotLocations] starting from `initial`. + pub(super) fn new(initial: Arc>) -> Self { + Self { + seen: vec![initial], + } + } + + /// Record that `hl` has been encountered during tracing. Return `true` if this indicates that + /// unrolling has occurred or `false` otherwise. + pub(super) fn push_and_check_unrolling(&mut self, hl: Arc>) -> bool { + let seen = self.seen.iter().skip(1).any(|x| Arc::ptr_eq(x, &hl)); + self.seen.push(hl); + seen + } +} + /// When a [HotLocation] has failed to compile a valid trace, should the [HotLocation] be tried /// again or not? #[derive(Debug)] diff --git a/ykrt/src/mt.rs b/ykrt/src/mt.rs index 5194ce28a..7cf168567 100644 --- a/ykrt/src/mt.rs +++ b/ykrt/src/mt.rs @@ -1,8 +1,9 @@ //! The main end-user interface to the meta-tracing system. use std::{ + assert_matches, cell::RefCell, - collections::{HashMap, HashSet}, + collections::HashMap, debug_assert_matches, env, error::Error, ffi::c_void, @@ -19,15 +20,18 @@ use parking_lot_core::SpinWait; use crate::{ aotsmp::{AOT_STACKMAPS, load_aot_stackmaps}, - compile::{CompilationError, CompiledTrace, Compiler, GuardId, default_compiler}, + compile::{ + CompilationError, CompiledTrace, Compiler, GuardId, Trace, TraceEnd, TraceStart, + default_compiler, + }, job_queue::{Job, JobQueue}, - location::{HotLocation, HotLocationKind, Location, TraceFailed}, + location::{HotLocation, HotLocationKind, Location, SeenHotLocations, TraceFailed}, log::{ Log, Verbosity, stats::{Stats, TimingState}, }, profile::{PlatformTraceProfiler, profiler_for_current_platform}, - trace::{AOTTraceIterator, TraceRecorder, Tracer, default_tracer}, + trace::{TraceRecorder, Tracer, default_tracer}, }; // Emit a log entry with hot location debug information if present and support is compiled in. @@ -260,6 +264,15 @@ impl MT { self.jit_enabled.load(Ordering::Relaxed) } + /// Return a reference to the [CompiledTrace] with ID `ctrid`. + /// + /// # Panics + /// + /// If `trid` is not in the set of compiled traces. + pub(crate) fn compiled_trace(self: &Arc, trid: TraceId) -> Arc { + Arc::clone(&self.compiled_traces.lock()[&trid]) + } + /// Return the unique ID for the next trace. pub(crate) fn next_trace_id(self: &Arc) -> TraceId { // Note: fetch_add is documented to wrap on overflow. @@ -272,58 +285,75 @@ impl MT { TraceId(ctr_id) } - /// Add a compilation job for a root trace where: - /// * `hl_arc` is the [HotLocation] this compilation job is related to. - /// * `ctrid` is the trace ID to be given to the new compiled trace. - /// * `connector_tid` is the optional trace ID of the trace the new compiled trace will - /// connect to. - fn queue_root_compile_job( - self: &Arc, - trace_iter: (Box, Box<[u8]>, Vec), - hl_arc: Arc>, - trid: TraceId, - connector_tid: Option, - ) { + /// Add `trace` to the compile queue. + fn queue_compile_job(self: &Arc, trace: Trace) { self.stats.trace_recorded_ok(); - let hl_arc_cl = Arc::clone(&hl_arc); + let (coupler_tid, failure): (_, Box) = { + let mt = Arc::clone(self); + match &trace.trace_start { + TraceStart::ControlPoint { hl } => { + let hl = Arc::clone(hl); + let ctrid = trace.ctrid; + let failure = move || { + let mut lk = hl.lock(); + debug_assert_matches!(lk.kind, HotLocationKind::Compiling(_)); + if let TraceFailed::DontTrace = lk.tracecompilation_error(&mt) { + lk.kind = HotLocationKind::DontTrace; + } else { + lk.kind = HotLocationKind::Counting(0); + } + mt.job_queue.notify_failure(&mt, ctrid); + }; + let coupler_tid = match &trace.trace_end { + TraceEnd::Loop => None, + TraceEnd::Coupler(coupler_tid) => Some(*coupler_tid), + }; + (coupler_tid, Box::new(failure)) + } + TraceStart::Guard { parent_ctr, gid } => { + let parent_ctr = Arc::clone(parent_ctr); + let gid = *gid; + let failure = move || parent_ctr.guard(gid).trace_or_compile_failed(&mt); + let TraceEnd::Coupler(coupler_tid) = &trace.trace_end else { + panic!() + }; + (Some(*coupler_tid), Box::new(failure)) + } + } + }; + let mt = Arc::clone(self); let main = move || { + mt.stats.timing_state(TimingState::Compiling); let compiler = { let lk = mt.compiler.lock(); Arc::clone(&*lk) }; - mt.stats.timing_state(TimingState::Compiling); - let connector_ctr = connector_tid.map(|x| Arc::clone(&mt.compiled_traces.lock()[&x])); - match compiler.root_compile( - Arc::clone(&mt), - trace_iter.0, - trid, - Arc::clone(&hl_arc), - trace_iter.1, - trace_iter.2, - connector_ctr, - ) { + let ctrid = trace.ctrid; + let trace_start = trace.trace_start.clone(); + + match compiler.compile(Arc::clone(&mt), trace) { Ok(ctr) => { - assert_eq!(ctr.ctrid(), trid); + assert_eq!(ctr.ctrid(), ctrid); mt.compiled_traces .lock() .insert(ctr.ctrid(), Arc::clone(&ctr)); - let mut hl = hl_arc.lock(); - debug_assert_matches!(hl.kind, HotLocationKind::Compiling(_)); - hl.kind = HotLocationKind::Compiled(ctr); mt.stats.trace_compiled_ok(); - mt.job_queue.notify_success(trid); + match trace_start { + TraceStart::ControlPoint { hl } => { + let mut lk = hl.lock(); + assert_matches!(lk.kind, HotLocationKind::Compiling(_)); + lk.kind = HotLocationKind::Compiled(ctr); + mt.job_queue.notify_success(ctrid); + } + TraceStart::Guard { parent_ctr, gid } => { + parent_ctr.guard(gid).set_ctr(ctr, &parent_ctr, gid); + } + } } Err(e) => { mt.stats.trace_compiled_err(); - let mut hl = hl_arc.lock(); - debug_assert_matches!(hl.kind, HotLocationKind::Compiling(_)); - if let TraceFailed::DontTrace = hl.tracecompilation_error(&mt) { - hl.kind = HotLocationKind::DontTrace; - } else { - hl.kind = HotLocationKind::Counting(0); - } match e { CompilationError::General(e) | CompilationError::LimitExceeded(e) => { mt.log.log( @@ -347,107 +377,19 @@ impl MT { .log(Verbosity::Error, &format!("trace-compilation-aborted: {e}")); } } - mt.job_queue.notify_failure(&mt, trid); - } - } - - mt.stats.timing_state(TimingState::None); - }; - - let mt = Arc::clone(self); - let failure = move || { - let mut hl = hl_arc_cl.lock(); - debug_assert_matches!(hl.kind, HotLocationKind::Compiling(_)); - if let TraceFailed::DontTrace = hl.tracecompilation_error(&mt) { - hl.kind = HotLocationKind::DontTrace; - } else { - hl.kind = HotLocationKind::Counting(0); - } - mt.job_queue.notify_failure(&mt, trid); - }; - - self.job_queue.push( - self, - Job::new(Box::new(main), connector_tid, Box::new(failure)), - ); - } - - /// Add a compilation job for a sidetrace where: `hl_arc` is the [HotLocation] this compilation - /// * `hl_arc` is the [HotLocation] this compilation job is related to. - /// * `root_ctr` is the root [CompiledTrace]. - /// * `parent_ctr` is the parent [CompiledTrace] of the side-trace that's about to be - /// compiled. Because side-traces can nest, this may or may not be the same [CompiledTrace] - /// as `root_ctr`. - /// * `guardid` is the ID of the guard in `parent_ctr` which failed. - /// * `connector_tid` is the optional trace ID of the trace the new compiled trace will - /// connect to. - fn queue_sidetrace_compile_job( - self: &Arc, - trace_iter: (Box, Box<[u8]>, Vec), - hl_arc: Arc>, - trid: TraceId, - parent_ctr: Arc, - gid: GuardId, - connector_tid: TraceId, - ) { - self.stats.trace_recorded_ok(); - let mt = Arc::clone(self); - let parent_ctr_cl = Arc::clone(&parent_ctr); - let main = move || { - let compiler = { - let lk = mt.compiler.lock(); - Arc::clone(&*lk) - }; - mt.stats.timing_state(TimingState::Compiling); - let target_ctr = Arc::clone(&mt.compiled_traces.lock()[&connector_tid]); - // FIXME: Can we pass in the root trace address, root trace entry variable locations, - // and the base stack-size from here, rather than spreading them out via - // DeoptInfo/SideTraceInfo, and CompiledTrace? - match compiler.sidetrace_compile( - Arc::clone(&mt), - trace_iter.0, - trid, - Arc::clone(&parent_ctr), - gid, - target_ctr, - Arc::clone(&hl_arc), - trace_iter.1, - trace_iter.2, - ) { - Ok(ctr) => { - assert_eq!(ctr.ctrid(), trid); - mt.compiled_traces - .lock() - .insert(ctr.ctrid(), Arc::clone(&ctr)); - parent_ctr.guard(gid).set_ctr(ctr, &parent_ctr, gid); - mt.stats.trace_compiled_ok(); - } - Err(e) => { - parent_ctr.guard(gid).trace_or_compile_failed(&mt); - mt.stats.trace_compiled_err(); - match e { - CompilationError::General(e) | CompilationError::LimitExceeded(e) => { - mt.log.log( - Verbosity::Warning, - &format!("sidetrace-compilation-aborted: {e}"), - ); - } - CompilationError::InternalError(e) => { - #[cfg(feature = "ykd")] - panic!("{e}"); - #[cfg(not(feature = "ykd"))] - { - mt.log.log( - Verbosity::Error, - &format!("sidetrace-compilation-aborted: {e}"), - ); + match trace_start { + TraceStart::ControlPoint { hl } => { + let mut lk = hl.lock(); + assert_matches!(lk.kind, HotLocationKind::Compiling(_)); + if let TraceFailed::DontTrace = lk.tracecompilation_error(&mt) { + lk.kind = HotLocationKind::DontTrace; + } else { + lk.kind = HotLocationKind::Counting(0); } + mt.job_queue.notify_failure(&mt, ctrid); } - CompilationError::ResourceExhausted(e) => { - mt.log.log( - Verbosity::Error, - &format!("sidetrace-compilation-aborted: {e}"), - ); + TraceStart::Guard { parent_ctr, gid } => { + parent_ctr.guard(gid).trace_or_compile_failed(&mt); } } } @@ -456,14 +398,8 @@ impl MT { mt.stats.timing_state(TimingState::None); }; - let mt = Arc::clone(self); - let failure = move || { - parent_ctr_cl.guard(gid).trace_or_compile_failed(&mt); - }; - self.job_queue.push( - self, - Job::new(Box::new(main), Some(connector_tid), Box::new(failure)), - ); + self.job_queue + .push(self, Job::new(Box::new(main), coupler_tid, failure)); } #[allow(clippy::not_unsafe_ptr_arg_deref)] @@ -515,19 +451,19 @@ impl MT { TransitionControlPoint::StartTracing(hl, trid) => { self.start_tracing(frameaddr, loc, hl, trid); } - TransitionControlPoint::StopTracing(trid, connector_tid) => { - self.stop_tracing(frameaddr, loc, trid, connector_tid); + TransitionControlPoint::StopTracing(trid, coupler_tid) => { + self.stop_tracing(frameaddr, loc, trid, coupler_tid); } TransitionControlPoint::StopSideTracing { trid, gid, parent_ctr, - connector_tid, + coupler_tid, start, } => { // Assuming no bugs elsewhere, the `unwrap`s cannot fail, because // `StartSideTracing` will have put a `Some` in the `Rc`. - let (hl, thread_tracer, promotions, debug_strs) = + let (thread_tracer, promotions, debug_strs) = MTThread::with_borrow_mut(|mtt| match mtt.pop_tstate() { MTThreadState::Tracing { trid: _, @@ -540,12 +476,13 @@ impl MT { gtrace: _, } => { assert_eq!(frameaddr, tracing_frameaddr); - (hl, thread_tracer, promotions, debug_strs) + assert!(Arc::ptr_eq(&hl, &parent_ctr.hl().upgrade().unwrap())); + (thread_tracer, promotions, debug_strs) } _ => unreachable!(), }); match thread_tracer.stop() { - Ok(utrace) => { + Ok(ta_iter) => { MTThread::set_tracing(IsTracing::None); self.stats.timing_state(TimingState::None); yklog!( @@ -554,20 +491,20 @@ impl MT { "stop-tracing", loc.hot_location() ); - self.queue_sidetrace_compile_job( - (utrace, promotions.into_boxed_slice(), debug_strs), - hl, - trid, - parent_ctr, - gid, - connector_tid, - ); + self.queue_compile_job(Trace { + trace_start: TraceStart::Guard { parent_ctr, gid }, + trace_end: TraceEnd::Coupler(coupler_tid), + ctrid: trid, + ta_iter, + promotions: promotions.into_boxed_slice(), + debug_strs, + }); if start { self.start_tracing( frameaddr, loc, loc.hot_location_arc_clone().unwrap(), - connector_tid, + coupler_tid, ); } } @@ -616,13 +553,13 @@ impl MT { match Arc::clone(&tracer).start_recorder() { Ok(tt) => { mtt.push_tstate(MTThreadState::Tracing { - hl, + hl: Arc::clone(&hl), trid, thread_tracer: tt, promotions: Vec::new(), debug_strs: Vec::new(), frameaddr, - seen_hls: HashSet::new(), + seen_hls: SeenHotLocations::new(hl), gtrace: None, }); } @@ -636,14 +573,14 @@ impl MT { }); } - /// Stop tracing of the trace with id `trid` at `loc`. If `connector_tid` is `Some`, the - /// resulting trace will be a connector trace. + /// Stop tracing of the trace with id `trid` at `loc`. If `coupler_tid` is `Some`, the + /// resulting trace will be a coupler trace. fn stop_tracing( self: &Arc, _frameaddr: *mut c_void, _loc: &Location, - trid: TraceId, - connector_tid: Option, + ctrid: TraceId, + coupler_tid: Option, ) { // Assuming no bugs elsewhere, the `unwrap`s cannot fail, because `StartTracing` // will have put a `Some` in the `Rc`. @@ -666,7 +603,7 @@ impl MT { _ => unreachable!(), }); match thread_tracer.stop() { - Ok(utrace) => { + Ok(ta_iter) => { MTThread::set_tracing(IsTracing::None); self.stats.timing_state(TimingState::None); yklog!( @@ -675,16 +612,22 @@ impl MT { "stop-tracing", _loc.hot_location() ); - self.queue_root_compile_job( - (utrace, promotions.into_boxed_slice(), debug_strs), - hl, - trid, - connector_tid, - ); + let trace_end = match coupler_tid { + Some(x) => TraceEnd::Coupler(x), + None => TraceEnd::Loop, + }; + self.queue_compile_job(Trace { + trace_start: TraceStart::ControlPoint { hl }, + trace_end, + ctrid, + ta_iter, + promotions: promotions.into_boxed_slice(), + debug_strs, + }); } Err(e) => { MTThread::set_tracing(IsTracing::None); - self.job_queue.notify_failure(self, trid); + self.job_queue.notify_failure(self, ctrid); self.stats.timing_state(TimingState::None); self.stats.trace_recorded_err(); yklog!( @@ -860,19 +803,12 @@ impl MT { } } - match loc.hot_location() { + match loc.hot_location_arc_clone() { Some(hl) => { - let mut akind = None; assert!(std::ptr::eq(frameaddr, *tracing_frameaddr)); - if let Some(x) = loc.hot_location().map(|x| x as *const Mutex) - && !seen_hls.insert(x) - { + if seen_hls.push_and_check_unrolling(Arc::clone(&hl)) { // We have traced this location more than once. - akind = Some(AbortKind::Unrolled); - } - - if let Some(akind) = akind { self.stats.trace_recorded_err(); let mut lk = tracing_hl.lock(); match &lk.kind { @@ -894,7 +830,7 @@ impl MT { } } - return TransitionControlPoint::AbortTracing(akind); + return TransitionControlPoint::AbortTracing(AbortKind::Unrolled); } let mut lk = hl.lock(); @@ -927,7 +863,8 @@ impl MT { } } None => { - let hl_ptr = match loc.inc_count() { + assert!(std::ptr::eq(frameaddr, *tracing_frameaddr)); + let hl = match loc.inc_count() { Some(count) => { let hl = HotLocation { kind: HotLocationKind::Counting(count), @@ -935,15 +872,13 @@ impl MT { debug_str: None, }; loc.count_to_hot_location(count, hl) - .map(|x| Arc::as_ptr(&x)) } - None => loc.hot_location().map(|x| x as *const Mutex), + None => loc.hot_location_arc_clone(), }; - if let Some(hl_ptr) = hl_ptr { - assert!(std::ptr::eq(frameaddr, *tracing_frameaddr)); - if !seen_hls.insert(hl_ptr) { - return TransitionControlPoint::AbortTracing(AbortKind::Unrolled); - } + if let Some(hl) = hl + && seen_hls.push_and_check_unrolling(hl) + { + return TransitionControlPoint::AbortTracing(AbortKind::Unrolled); } TransitionControlPoint::NoAction } @@ -1004,7 +939,7 @@ impl MT { HotLocationKind::Compiled(_) | HotLocationKind::Compiling(_) | HotLocationKind::Tracing(_) => { - let connector_tid = match lk.kind { + let coupler_tid = match lk.kind { HotLocationKind::Compiled(ref ctr) => ctr.ctrid(), HotLocationKind::Compiling(ref ctrid) => *ctrid, HotLocationKind::Tracing(tid) => tid, @@ -1020,7 +955,7 @@ impl MT { trid: *tracing_trid, gid, parent_ctr, - connector_tid, + coupler_tid, start: false, } } @@ -1037,7 +972,7 @@ impl MT { trid: *tracing_trid, gid, parent_ctr, - connector_tid: next_tid, + coupler_tid: next_tid, start: true, } } @@ -1045,7 +980,13 @@ impl MT { } } None => { - let hl_ptr = match loc.inc_count() { + if !std::ptr::eq(frameaddr, *tracing_frameaddr) { + // We're tracing but no longer in the frame we started in, so we + // need to stop tracing and report the original [HotLocation] as + // having failed to trace properly. + return TransitionControlPoint::AbortTracing(AbortKind::OutOfFrame); + } + let hl = match loc.inc_count() { Some(count) => { let hl = HotLocation { kind: HotLocationKind::Counting(count), @@ -1053,20 +994,13 @@ impl MT { debug_str: None, }; loc.count_to_hot_location(count, hl) - .map(|x| Arc::as_ptr(&x)) } - None => loc.hot_location().map(|x| x as *const Mutex), + None => loc.hot_location_arc_clone(), }; - if let Some(hl_ptr) = hl_ptr { - if !std::ptr::eq(frameaddr, *tracing_frameaddr) { - // We're tracing but no longer in the frame we started in, so we - // need to stop tracing and report the original [HotLocation] as - // having failed to trace properly. - return TransitionControlPoint::AbortTracing(AbortKind::OutOfFrame); - } - if !seen_hls.insert(hl_ptr) { - return TransitionControlPoint::AbortTracing(AbortKind::Unrolled); - } + if let Some(hl) = hl + && seen_hls.push_and_check_unrolling(hl) + { + return TransitionControlPoint::AbortTracing(AbortKind::Unrolled); } TransitionControlPoint::NoAction } @@ -1190,12 +1124,12 @@ impl MT { MTThread::with_borrow_mut(|mtt| match Arc::clone(&tracer).start_recorder() { Ok(tt) => mtt.push_tstate(MTThreadState::Tracing { trid, - hl, + hl: Arc::clone(&hl), thread_tracer: tt, promotions: Vec::new(), debug_strs: Vec::new(), frameaddr, - seen_hls: HashSet::new(), + seen_hls: SeenHotLocations::new(hl), gtrace: Some((parent, gid)), }), Err(e) => { @@ -1242,7 +1176,7 @@ enum MTThreadState { /// the time being we force every `Location` that we encounter in a trace to become a /// [HotLocation] (with kind [HotLocationKind::Counting]) if it is not already. We can then /// use the (unmoving) pointer to a [HotLocation]'s inner [Mutex] as an ID. - seen_hls: HashSet<*const Mutex>, + seen_hls: SeenHotLocations, /// The [HotLocation] the trace will end at. For a top-level trace, this will be the same /// [HotLocation] the trace started at; for a side-trace, tracing started elsewhere. hl: Arc>, @@ -1358,10 +1292,10 @@ impl MTThread { /// /// If the stack is empty. There should always be at least one element on the stack, so a panic /// here means that something has gone wrong elsewhere. - pub(crate) fn compiled_trace(&self, ctrid: TraceId) -> Arc { + pub(crate) fn compiled_trace(&self, trid: TraceId) -> Arc { for tstate in self.tstate.iter().rev() { if let MTThreadState::Executing { mt } = tstate { - return Arc::clone(&mt.compiled_traces.lock()[&ctrid]); + return mt.compiled_trace(trid); } } panic!(); @@ -1519,14 +1453,14 @@ enum TransitionControlPoint { /// use the `Arc` to detect tracing issues in other threads, and we need to keep it alive for /// the duration of the transition calls for that to work properly. StartTracing(Arc>, TraceId), - /// Stop tracing. If `Option` is not-`None`, we have a connector trace. + /// Stop tracing. If `Option` is not-`None`, we have a coupler trace. StopTracing(TraceId, Option), /// Stop side tracing. StopSideTracing { trid: TraceId, gid: GuardId, parent_ctr: Arc, - connector_tid: TraceId, + coupler_tid: TraceId, // Should a new trace be immediately started after the guard trace? start: bool, }, @@ -1597,7 +1531,7 @@ mod tests { use super::*; use crate::{ compile::{CompiledTraceTestingBasicTransitions, CompiledTraceTestingMinimal}, - trace::TraceRecorderError, + trace::{AOTTraceIterator, TraceRecorderError}, }; use std::{assert_matches, hint::black_box, ptr, thread}; use test::bench::Bencher; @@ -1638,12 +1572,12 @@ mod tests { MTThread::with_borrow_mut(|mtt| { mtt.push_tstate(MTThreadState::Tracing { trid, - hl, + hl: Arc::clone(&hl), thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), debug_strs: Vec::new(), frameaddr: ptr::null_mut(), - seen_hls: HashSet::new(), + seen_hls: SeenHotLocations::new(hl), gtrace: None, }); }); @@ -1672,12 +1606,12 @@ mod tests { MTThread::with_borrow_mut(|mtt| { mtt.push_tstate(MTThreadState::Tracing { trid, - hl, + hl: Arc::clone(&hl), thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), debug_strs: Vec::new(), frameaddr: ptr::null_mut(), - seen_hls: HashSet::new(), + seen_hls: SeenHotLocations::new(hl), gtrace: Some((ctr, GuardId::from(0))), }); }); @@ -1801,12 +1735,12 @@ mod tests { MTThread::with_borrow_mut(|mtt| { mtt.push_tstate(MTThreadState::Tracing { trid, - hl, + hl: Arc::clone(&hl), thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), debug_strs: Vec::new(), frameaddr: ptr::null_mut(), - seen_hls: HashSet::new(), + seen_hls: SeenHotLocations::new(hl), gtrace: None, }); }); @@ -2040,12 +1974,12 @@ mod tests { MTThread::with_borrow_mut(|mtt| { mtt.push_tstate(MTThreadState::Tracing { trid, - hl, + hl: Arc::clone(&hl), thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), debug_strs: Vec::new(), frameaddr: ptr::null_mut(), - seen_hls: HashSet::new(), + seen_hls: SeenHotLocations::new(hl), gtrace: None, }); });