Skip to content

Commit 648241e

Browse files
bors[bot]michalt
andauthored
Merge #2878
2878: Fix a corner case when printing unaccounted time r=matklad a=michalt Previously `ra_prof` wouldn't actually print the unaccounted time in some cases. We would print, for instance, this: ``` 5ms - foo 2ms - bar ``` instead of: ``` 5ms - foo 2ms - bar 3ms - ??? ``` The fix is to properly handle the case when an entry has 0 children instead of using the `last` variable. Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com> Co-authored-by: Michal Terepeta <michal.terepeta@gmail.com>
2 parents 3a7724e + e8acf49 commit 648241e

File tree

1 file changed

+52
-4
lines changed

1 file changed

+52
-4
lines changed

crates/ra_prof/src/lib.rs

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,11 @@ fn print(
219219
longer_than: Duration,
220220
total: Option<Duration>,
221221
) {
222-
let mut last = 0;
222+
if msgs.is_empty() {
223+
return;
224+
}
225+
// The index of the first element that will be included in the slice when we recurse.
226+
let mut next_start = 0;
223227
let indent = repeat(" ").take(lvl).collect::<String>();
224228
// We output hierarchy for long calls, but sum up all short calls
225229
let mut short = Vec::new();
@@ -233,12 +237,12 @@ fn print(
233237
writeln!(out, "{}{:5}ms - {}", indent, duration.as_millis(), msg)
234238
.expect("printing profiling info to stdout");
235239

236-
print(lvl + 1, &msgs[last..i], out, longer_than, Some(duration));
240+
print(lvl + 1, &msgs[next_start..i], out, longer_than, Some(duration));
237241
} else {
238242
short.push((msg, duration))
239243
}
240244

241-
last = i;
245+
next_start = i + 1;
242246
}
243247
short.sort_by_key(|(msg, _time)| *msg);
244248
for (msg, entires) in short.iter().group_by(|(msg, _time)| msg).into_iter() {
@@ -255,7 +259,7 @@ fn print(
255259
if let Some(total) = total {
256260
if let Some(unaccounted) = total.checked_sub(accounted_for) {
257261
let unaccounted_millis = unaccounted.as_millis();
258-
if unaccounted_millis > longer_than.as_millis() && unaccounted_millis > 0 && last > 0 {
262+
if unaccounted_millis > longer_than.as_millis() && unaccounted_millis > 0 {
259263
writeln!(out, "{}{:5}ms - ???", indent, unaccounted_millis)
260264
.expect("printing profiling info to stdout");
261265
}
@@ -377,4 +381,48 @@ mod tests {
377381
" 1ms - foo\n 0ms - bar (2 calls)\n"
378382
);
379383
}
384+
385+
#[test]
386+
fn test_unaccounted_for_topmost() {
387+
let mut result = vec![];
388+
let msgs = vec![
389+
Message { level: 1, duration: Duration::from_millis(2), message: "bar".to_owned() },
390+
Message { level: 0, duration: Duration::from_millis(5), message: "foo".to_owned() },
391+
];
392+
print(0, &msgs, &mut result, Duration::from_millis(0), Some(Duration::from_millis(1)));
393+
assert_eq!(
394+
std::str::from_utf8(&result).unwrap().lines().collect::<Vec<_>>(),
395+
vec![
396+
" 5ms - foo",
397+
" 2ms - bar",
398+
" 3ms - ???",
399+
// Dummy comment to improve formatting
400+
]
401+
);
402+
}
403+
404+
#[test]
405+
fn test_unaccounted_for_multiple_levels() {
406+
let mut result = vec![];
407+
let msgs = vec![
408+
Message { level: 2, duration: Duration::from_millis(3), message: "baz".to_owned() },
409+
Message { level: 1, duration: Duration::from_millis(5), message: "bar".to_owned() },
410+
Message { level: 2, duration: Duration::from_millis(2), message: "baz".to_owned() },
411+
Message { level: 1, duration: Duration::from_millis(4), message: "bar".to_owned() },
412+
Message { level: 0, duration: Duration::from_millis(9), message: "foo".to_owned() },
413+
];
414+
print(0, &msgs, &mut result, Duration::from_millis(0), None);
415+
assert_eq!(
416+
std::str::from_utf8(&result).unwrap().lines().collect::<Vec<_>>(),
417+
vec![
418+
" 9ms - foo",
419+
" 5ms - bar",
420+
" 3ms - baz",
421+
" 2ms - ???",
422+
" 4ms - bar",
423+
" 2ms - baz",
424+
" 2ms - ???",
425+
]
426+
);
427+
}
380428
}

0 commit comments

Comments
 (0)