Skip to content

Commit

Permalink
Exclude system timers from runtime stats (#785)
Browse files Browse the repository at this point in the history
  • Loading branch information
littledivy authored Jun 18, 2024
1 parent 81bd99d commit e0f2036
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 18 deletions.
10 changes: 3 additions & 7 deletions core/ops_builtin_v8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,9 @@ pub fn op_timer_queue_system(
#[global] task: v8::Global<v8::Function>,
) -> f64 {
let context_state = JsRealm::state_from_scope(scope);
if repeat {
context_state
.timers
.queue_timer_repeat(timeout_ms as _, (task, 0)) as _
} else {
context_state.timers.queue_timer(timeout_ms as _, (task, 0)) as _
}
context_state
.timers
.queue_system_timer(repeat, timeout_ms as _, (task, 0)) as _
}

/// Queue a timer. We return a "large integer" timer ID in an f64 which allows for up
Expand Down
9 changes: 8 additions & 1 deletion core/runtime/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,14 @@ impl RuntimeActivityStatsFactory {
timers: Vec::with_capacity(timer_count),
repeats: BitSet::with_capacity(timer_count),
};
for (timer_id, repeats) in &self.context_state.timers.iter() {
for (timer_id, repeats, is_system_timer) in
&self.context_state.timers.iter()
{
// Ignore system timer from stats
if is_system_timer {
continue;
}

if repeats {
timers.repeats.insert(timers.timers.len());
}
Expand Down
31 changes: 21 additions & 10 deletions core/web_timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ enum TimerType {
}

#[derive(PartialEq, Eq, PartialOrd, Ord)]
struct TimerKey(Instant, u64, TimerType);
struct TimerKey(Instant, u64, TimerType, bool);

struct TimerData<T> {
data: T,
Expand Down Expand Up @@ -106,7 +106,7 @@ pub(crate) struct WebTimersIterator<'a, T> {

impl<'a, T> IntoIterator for &'a WebTimersIterator<'a, T> {
type IntoIter = WebTimersIteratorImpl<'a, T>;
type Item = (u64, bool);
type Item = (u64, bool, bool);

fn into_iter(self) -> Self::IntoIter {
WebTimersIteratorImpl {
Expand All @@ -122,12 +122,12 @@ pub(crate) struct WebTimersIteratorImpl<'a, T> {
}

impl<'a, T> Iterator for WebTimersIteratorImpl<'a, T> {
type Item = (u64, bool);
type Item = (u64, bool, bool);
fn next(&mut self) -> Option<Self::Item> {
loop {
let item = self.timers.next()?;
if self.data.contains_key(&item.1) {
return Some((item.1, !matches!(item.2, TimerType::Once)));
return Some((item.1, !matches!(item.2, TimerType::Once), item.3));
}
}
}
Expand Down Expand Up @@ -287,19 +287,29 @@ impl<T: Clone> WebTimers<T> {

/// Queues a timer to be fired in order with the other timers in this set of timers.
pub fn queue_timer(&self, timeout_ms: u64, data: T) -> WebTimerId {
self.queue_timer_internal(false, timeout_ms, data)
self.queue_timer_internal(false, timeout_ms, data, false)
}

/// Queues a timer to be fired in order with the other timers in this set of timers.
pub fn queue_timer_repeat(&self, timeout_ms: u64, data: T) -> WebTimerId {
self.queue_timer_internal(true, timeout_ms, data)
self.queue_timer_internal(true, timeout_ms, data, false)
}

pub fn queue_system_timer(
&self,
repeat: bool,
timeout_ms: u64,
data: T,
) -> WebTimerId {
self.queue_timer_internal(repeat, timeout_ms, data, true)
}

fn queue_timer_internal(
&self,
repeat: bool,
timeout_ms: u64,
data: T,
is_system_timer: bool,
) -> WebTimerId {
#[allow(clippy::let_unit_value)]
let high_res = self.high_res_timer_lock.maybe_lock(timeout_ms);
Expand All @@ -326,7 +336,7 @@ impl<T: Clone> WebTimers<T> {
} else {
TimerType::Once
};
timers.insert(TimerKey(deadline, id, timer_type));
timers.insert(TimerKey(deadline, id, timer_type, is_system_timer));

let mut data_map = self.data_map.borrow_mut();
data_map.insert(
Expand Down Expand Up @@ -380,9 +390,9 @@ impl<T: Clone> WebTimers<T> {
let mut data = self.data_map.borrow_mut();
let mut output = vec![];

let mut split = timers.split_off(&TimerKey(now, 0, TimerType::Once));
let mut split = timers.split_off(&TimerKey(now, 0, TimerType::Once, false));
std::mem::swap(&mut split, &mut timers);
for TimerKey(_, id, timer_type) in split {
for TimerKey(_, id, timer_type, is_system_timer) in split {
if let TimerType::Repeat(interval) = timer_type {
if let Some(TimerData { data, .. }) = data.get(&id) {
output.push((id, data.clone()));
Expand All @@ -392,6 +402,7 @@ impl<T: Clone> WebTimers<T> {
.unwrap(),
id,
timer_type,
is_system_timer,
));
}
} else if let Some(TimerData {
Expand All @@ -413,7 +424,7 @@ impl<T: Clone> WebTimers<T> {
// We should never have an ineffective poll when the data map is empty, as we check
// for this in cancel_timer.
debug_assert!(!data.is_empty());
while let Some(TimerKey(_, id, _)) = timers.first() {
while let Some(TimerKey(_, id, ..)) = timers.first() {
if data.contains_key(id) {
break;
} else {
Expand Down
17 changes: 17 additions & 0 deletions testing/unit/stats_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,20 @@ test(async function testIntervalLeakTrace() {
assertEquals(tracesFinal.size, 0);
});
});

test(async function testSystemTimeoutLeakTrace() {
await enableTracingForTest(() => {
const tracesBefore = Deno.core.getAllLeakTraces();
using statsBefore = StatsFactory.capture();
const t1 = Deno.core.queueSystemTimer(undefined, false, 100_000, () => {});
const tracesAfter = Deno.core.getAllLeakTraces();
using statsAfter = StatsFactory.capture();
const diff = StatsFactory.diff(statsBefore, statsAfter);
assertEquals(diff.appeared.countWithTraces(LeakType.Timer), 0);

assertEquals(tracesAfter.size, tracesBefore.size);
clearTimeout(t1);
const tracesFinal = Deno.core.getAllLeakTraces();
assertEquals(tracesFinal.size, 0);
});
});

0 comments on commit e0f2036

Please sign in to comment.