Skip to content

Commit 468fa2c

Browse files
committed
Address review comments and remove fencing from rdtsc()
1 parent 1bede14 commit 468fa2c

File tree

1 file changed

+8
-27
lines changed

1 file changed

+8
-27
lines changed

src/bin/log/tracing_chrome_instant.rs

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
//! since the former is not reliable (i.e. it might lead to non-monotonic time measurements).
77
//! Another useful resource for future improvements might be measureme's time measurement utils:
88
//! <https://github.com/rust-lang/measureme/blob/master/measureme/src/counters.rs>.
9+
//! Documentation about how the Linux kernel chooses a clock source can be found here:
10+
//! <https://btorpey.github.io/blog/2014/02/18/clock-sources-in-linux/>.
911
#![cfg(feature = "tracing")]
1012

1113
/// This alternative `TracingChromeInstant` implementation was made entirely to suit the needs of
@@ -90,27 +92,16 @@ impl TracingChromeInstant {
9092
f(ts);
9193

9294
// Measure how much time was spent executing `f` and shift `start_instant` forward
93-
// by that amount.
95+
// by that amount. This "removes" that time from the trace.
9496
*start_instant += std::time::Instant::now() - instant_before_f;
9597
}
9698

9799
#[cfg(all(target_os = "linux", any(target_arch = "x86", target_arch = "x86_64")))]
98100
TracingChromeInstant::Tsc { start_tsc, tsc_to_microseconds } => {
99-
// Obtain the current time (before executing `f`).
101+
// the comments above also apply here, since it's the same logic
100102
let tsc_before_f = tsc::rdtsc();
101-
102-
// Using the current time (`instant_before_f`) and the `start_instant` stored in
103-
// `self`, calculate the elapsed time (in microseconds) since this instant was
104-
// instantiated, accounting for any time that was previously spent executing `f`.
105-
// The "accounting" part is not computed in this line, but is rather done by
106-
// shifting forward the `start_instant` down below.
107103
let ts = ((tsc_before_f - *start_tsc) as f64) * (*tsc_to_microseconds);
108-
109-
// Run the function (supposedly a function internal to the tracing infrastructure).
110104
f(ts);
111-
112-
// Measure how much time was spent executing `f` and shift `start_instant` forward
113-
// by that amount.
114105
*start_tsc += tsc::rdtsc() - tsc_before_f;
115106
}
116107
}
@@ -127,21 +118,11 @@ mod tsc {
127118
#[inline(always)]
128119
pub(super) fn rdtsc() -> u64 {
129120
#[cfg(target_arch = "x86")]
130-
use core::arch::x86::{_mm_lfence, _rdtsc};
121+
use core::arch::x86::_rdtsc;
131122
#[cfg(target_arch = "x86_64")]
132-
use core::arch::x86_64::{_mm_lfence, _rdtsc};
133-
use std::arch::asm;
134-
135-
unsafe {
136-
// Fence the execution with `_mm_lfence` and `asm!` to ensure that neither the compiler
137-
// nor the CPU reorder instructions, as that would make _rdtsc() run at the wrong time.
138-
_mm_lfence();
139-
asm!("", options(nostack, preserves_flags));
140-
let tsc = _rdtsc();
141-
asm!("", options(nostack, preserves_flags));
142-
_mm_lfence();
143-
tsc
144-
}
123+
use core::arch::x86_64::_rdtsc;
124+
125+
unsafe { _rdtsc() }
145126
}
146127

147128
/// Estimates the frequency of the TSC counter by waiting 10ms in a busy loop and

0 commit comments

Comments
 (0)