Skip to content

Commit a4b11f4

Browse files
committed
fix(callgrind-utils): parse sparse and instr-line cost lines
The cost-line parser required exactly `num_positions + num_events` tokens, but real Callgrind output uses `positions: instr line` (two position columns) and omits trailing zero event counts, so cost lines are variable-length. Every real cost line was therefore rejected, leaving all self costs at zero and the flamegraph empty for actual profiles (rust/cpp/node samples all folded to 0). Read the first event (Ir) at token index `num_positions`, accepting 1..=num_events trailing counts, and validate the leading tokens as Callgrind position tokens (`*`, `0x..`, absolute, or `+N`/`-N`) to keep rejecting colon headers.
1 parent d004116 commit a4b11f4

2 files changed

Lines changed: 52 additions & 9 deletions

File tree

callgrind-utils/src/parser.rs

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -265,18 +265,34 @@ fn header_token_count(trimmed: &str, key: &str) -> usize {
265265

266266
/// First event value of a cost line, or `None` if `trimmed` is not one.
267267
///
268-
/// A cost line has exactly `num_positions + num_events` whitespace-separated
269-
/// tokens; the position tokens (line/instr, possibly `+N`/`-N`/`*`/`0x..`) are
270-
/// ignored and the first event column (token index `num_positions`) is parsed
271-
/// as a decimal count. The strict token-count + decimal-parse check rejects
272-
/// colon headers and bare tokens that also lack an `=`.
268+
/// A cost line is `num_positions` position tokens followed by 1..=`num_events`
269+
/// event counts; Callgrind omits trailing zero counts, so the value list is
270+
/// variable-length. The first event column (`Ir`, token index `num_positions`)
271+
/// is returned. Requiring the leading tokens to be position-like (line/instr,
272+
/// possibly `+N`/`-N`/`*`/`0x..`) plus a decimal first value rejects colon
273+
/// headers and bare tokens that also lack an `=`.
273274
fn parse_cost_value(trimmed: &str, num_positions: usize, num_events: usize) -> Option<u64> {
274-
let mut tokens = trimmed.split_whitespace();
275-
let count = trimmed.split_whitespace().count();
276-
if count != num_positions + num_events {
275+
let tokens: Vec<&str> = trimmed.split_whitespace().collect();
276+
if tokens.len() <= num_positions || tokens.len() > num_positions + num_events {
277277
return None;
278278
}
279-
tokens.nth(num_positions)?.parse::<u64>().ok()
279+
if !tokens[..num_positions].iter().all(|t| is_position_token(t)) {
280+
return None;
281+
}
282+
tokens[num_positions].parse::<u64>().ok()
283+
}
284+
285+
/// Whether `tok` is a Callgrind position/subposition token: `*` (repeat), an
286+
/// absolute decimal or `0x` address, or a `+N`/`-N` relative offset.
287+
fn is_position_token(tok: &str) -> bool {
288+
if tok == "*" {
289+
return true;
290+
}
291+
if let Some(hex) = tok.strip_prefix("0x").or_else(|| tok.strip_prefix("0X")) {
292+
return !hex.is_empty() && hex.bytes().all(|b| b.is_ascii_hexdigit());
293+
}
294+
let digits = tok.strip_prefix(['+', '-']).unwrap_or(tok);
295+
!digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit())
280296
}
281297

282298
/// The leading token of `line`: everything up to the first `=`, `:`, or

callgrind-utils/tests/flamegraph.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,33 @@ fn heavy_frame_behind_zero_cost_edge_survives() {
145145
assert_eq!(total, 105, "entry(5) + hot(100)");
146146
}
147147

148+
/// Real Callgrind uses `positions: instr line` (two position columns) and omits
149+
/// trailing zero event counts, so cost lines are variable-length. The parser
150+
/// must read the first event (`Ir`) at token index `num_positions` regardless
151+
/// of how many trailing counts are present, for both self and inclusive costs.
152+
const SPARSE: &str = "\
153+
positions: instr line
154+
events: Ir Dr Dw
155+
fn=main
156+
0x1000 10 7
157+
+4 11 3 0
158+
cfn=leaf
159+
calls=1 0 0
160+
0x2000 12 20
161+
fn=leaf
162+
0x3000 20 20 0 0
163+
";
164+
165+
#[test]
166+
fn parses_sparse_instr_line_cost_lines() {
167+
let g = parse(SPARSE);
168+
assert_eq!(
169+
folded_sorted(&g),
170+
vec!["main 10".to_string(), "main;leaf 20".to_string()],
171+
"self=7+3 for main, inclusive/self=20 for leaf"
172+
);
173+
}
174+
148175
#[test]
149176
fn no_cost_data_is_an_error() {
150177
// Topology only, no cost columns beyond position -> every cost is zero.

0 commit comments

Comments
 (0)