search

jlfwong / speedscope

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

Partition based on samples instead of traceEvents when importing a sample-based chrome trace #460

Closed zacharyfmarion closed 8 months ago

zacharyfmarion commented 8 months ago

In the existing code, if traceEvents did not have any importable events, the profile would be marked as empty. This was a bug, as the dev Hermes profiles I was testing with had one X event which made the code work. However we do not need to guarantee (and the spec doesn't seem to) any traceEvents being present as long as we have samples and stack frames.

When I tested a production profile taken from Hermes it did not have any importable events, just a metadata event with a thread name. This PR updates the implementation to iterate over partitioned samples instead of traceEvents so we construct profiles for all thread + process pairs referenced in the samples array.

Before After
Screenshot 2024-01-03 at 9 58 56 AM Screenshot 2024-01-03 at 9 58 17 AM
coveralls commented 8 months ago

Coverage Status

coverage: 43.608% (+0.08%) from 43.524% when pulling 8be44afa506388b6ca375d3db67a5427c0853cf3 on zacharyfmarion:zac/use-samples into a3c0b15935fe125130ef371ec79dbe1c5e744b63 on jlfwong:main.

zacharyfmarion commented 7 months ago

@jlfwong any chance you could release a new version for this? I'm starting to let people know they can use speedscope for production Hermes profiles here. Will probably get a tweet together once the issue is closed out and this PR lands

jlfwong commented 7 months ago

@zacharyfmarion Done!

zacharyfmarion commented 7 months ago

@jfwong thank you!