jlfwong / speedscope

🔬 A fast, interactive web-based viewer for performance profiles.
https://www.speedscope.app
MIT License
5.58k stars 247 forks source link

Callgrind parser: Fixed associating file names with symbols and detection of root nodes #466

Open glatosinski opened 8 months ago

glatosinski commented 8 months ago

This PR introduces several small improvements to updating the current file name for the Callgrind parser:

Those small modifications improve detection of root nodes. Before the PR, the same function could have several files associated with it - when fi/fe are not taken into account, the file name for the current callee is taken from the last fl or cfi call, which is later added to childMap in this.childrenTotalWeights instead of the actual function with its file name. After this, only one of the two representations of the same symbol get deleted from rootNodes.

Without the improvement, we can observe detached call stacks for such "orphaned" functions. With the improvement, the functions have properly provided names, which leads to better generation of the call graph.

Some examples before and after applying the commit from the PR (the marked calls are for the same function):

This fixes https://github.com/jlfwong/speedscope/issues/465

coveralls commented 8 months ago

Coverage Status

coverage: 43.652% (+0.03%) from 43.619% when pulling 20bc6cebee16a7cb349d58d3ea5a0af822fcdaf5 on antmicro:glat/callgrind-filename-parsing-fix into 0121cf93423033f41b42fc607ef076c7b8f8ac4e on jlfwong:main.

jlfwong commented 7 months ago

Hi @glatosinski!

Thanks for contributing this, and sorry for the long response time. Can you please add a minimal test that demonstrates the failure mode this PR fixes?

Thanks!

glatosinski commented 7 months ago

Hi, ok, I will prepare a minimal test