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

Fix bug in selectQueueToTakeFromNext for trace profiles #450

Closed zacharyfmarion closed 8 months ago

zacharyfmarion commented 8 months ago

I have been taking a lot of profiles using the Hermes profiler, but I noticed that they sometimes to not show up properly. After debugging what exactly was going on, I realized it was because the logic in selectQueueToTakeFromNext only checks for name, instead of the key for the event. I had a bunch of events with the name anonymous that were getting improperly exited before they should have been due to this logic.

This fix makes the code more robust if there are added "args" which differentiate an event from another (as is the case in Hermes profiles), however it would still be an issue if they key just defaults to the name.

Example profile before:

Screenshot 2023-12-15 at 12 54 04 AM

What it should look like (in Perfetto):

Screenshot 2023-12-15 at 8 51 38 AM

After the fix:

Screenshot 2023-12-15 at 12 54 29 AM
zacharyfmarion commented 8 months ago

@jlfwong can you take a look? I can provide an example trace but I don't want to post it publicly.

coveralls commented 8 months ago

Coverage Status

coverage: 43.041%. remained the same when pulling 447fdad502086f96369c80cc7ec54dca0f2a840a on zacharyfmarion:zac/fix-trace-bug into ac4a0155596da13f3a76d752d63d06a31dc32ac0 on jlfwong:main.

jlfwong commented 8 months ago

Hey @zacharyfmarion! Thanks for the investigation and the patch!

An example trace sent privately would be appreciated (you can send to jamie.lf.wong at gmail), but even better would be a dramatically reduced test file that minimally reproduces the issue.