jlfwong / jlfwong/speedscope

Callgrind import broken for profiles with `positions: instr line`

Open
#550 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
6.8k
Forks
320
PR merge metrics
No merged PRs in 30d

Description

Callgrind files produced by Valgrind with instruction-level profiling declare:

positions: instr line

In this instance, each cost line has two position fields; instruction address and line
number before the actual cost values.

Example:

0x4d10 408 1042

The parser hardcoded `numPositionFields = 1`, so it treated the line number
column as a cost value. This causes subsequent subposition-compressed lines
(e.g. `* * 1042`) to be misaligned, resulting in the following error when loading the file.

Failed to load format Error: Unrecognized line "* * " on line 136 parse

**To reproduce:**
Load any callgrind file that contains `positions: instr line`
in the header (e.g. output from `valgrind --tool=callgrind --collect-jumps=yes`).

I do have a patch ready and working with my profiling results. The fix includes the following:

- Parses the `positions:` header to set the correct number of position fields
- Handles hex instruction addresses (e.g. `0x4d10`)
- Handles jump specification lines (`jcnd`, `jump`, `jfi`, `jfl`, `jfn`)
- Resolves two existing TODOs in `parseCostLine`

Happy to open a PR if this issue is approved. Fix is currently available here with `npm run jest -- --testPathPatterns=callgrind` tests passing.

[https://github.com/InkbridgeNetworks/speedscope/tree/callgrind-import-patch](https://github.com/InkbridgeNetworks/speedscope/tree/callgrind-import-patch)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.