jlfwong / speedscope

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

Support showing pprof lines from the pprof Line object #409

Closed dalehamel closed 1 year ago

dalehamel commented 2 years ago

The previous behavior was to use the StartLine of a function as the line number to show in speedscope. However, the Line object has more precise line information, and we should only fallback to StartLine if we don't have this more detailed information.

Looking at the documentation for the pprof proto, this is more how it intends to interpret line information:

location: A unique place in the program, commonly mapped to a single instruction address. It has a unique nonzero id, to be referenced from the samples. It contains source information in the form of lines, and a mapping id that points to a binary. function: A program function as defined in the program source. It has a unique nonzero id, referenced from the location lines. It contains a human-readable name for the function (eg a C++ demangled name), a system name (eg a C++ mangled name), the name of the corresponding source file, and other function attributes.

Here is a sample profile that had line-level info on the Line object of the profile:

Before:

Screen Shot 2022-11-03 at 11 17 11 AM

After:

Screen Shot 2022-11-03 at 11 17 22 AM

NB i have never written anything in typscript before, but I tried to follow CONTRIBUTING.md, and run the necessary linter and tests. Note also that the linter changed other files which I didn't modify, so I didn't commit those changes.

dalehamel commented 2 years ago

@jlfwong when you have the time to review

dalehamel commented 1 year ago

Status is that this seems to work, it fixes the previous issue we had with the test fixtures, but now there is an import error that fails tests.

jlfwong commented 1 year ago

Okay, thanks! I'll take it from here -- I'll fix this on another branch. Thanks for the followthrough

jlfwong commented 1 year ago

This is now live on https://speedscope.app and published to npm as part of v1.15.2. Thanks for your contribution!

dalehamel commented 1 year ago

thanks so much for the assistance @jlfwong i really appreciate it! we can get off of our fork now i think ❤️