firefox-devtools / profiler

Firefox Profiler — Web app for Firefox performance analysis
https://profiler.firefox.com
Mozilla Public License 2.0
1.22k stars 398 forks source link

Option for custom marker ordering #5152

Open joshuay03 opened 1 month ago

joshuay03 commented 1 month ago

The vernier CRuby profiler uses a fork of the Firefox profiler as it's viewer, and I thought I'd first check if this feature would be generally useful before possibly implementing it in the fork.

I'm trying to order markers within each category/bucket without changing their names. Currently markers are sorted by name here. There's an example of what I'm trying to achieve in the description of https://github.com/jhawthorn/vernier/pull/98 where I tried to hack it into vernier's profile output without making changes to the viewer.

I would like to be able to provide a marker attribute like 'sortIndex' along with the name and category such that the sorting is performed by category first, then this index if provided else the name.

┆Issue is synchronized with this Jira Task

fqueze commented 1 month ago

Just to verify if I understand the suggestion correctly, you would like to set the marker names to eg. GC1$$GC start GC2$$GC pause GC3$$GC end

And have the profiler sort in the marker chart based on the full name, but strip anything before the '$$' (or any other arbitrary string we don't expect to ever find in a real marker name) before printing the names in the UI?

I think we could probably find some use for that in Firefox too, but it would probably remain a niche feature (as it would be hard to make it discoverable).

julienw commented 1 month ago

I think they would like to avoid doing that, by introducing another field instead.

It's been a long time I wanted to experiment with sorting mechanisms. The first one I'd like to try is sorting by startTime (either ignoring the categories or inside a category), having all markers in the same bucket, but then with (for example) colors to distinguish the various names. I think this could help with the problem of finding the causality of some events compared to another one.

If we really want to introduce an extra item for the default sort, then I think it should be specified in the marker schema: (maybe) sortProperty would designate which property inside the payload would be used to sort.

Thoughts?

joshuay03 commented 1 month ago

I think they would like to avoid doing that, by introducing another field instead.

Yes that's right. Although, @fqueze's prefix suggestion works too. As long as the final labels are in the format of GC start, and they're ordered as expected 🙂

If we really want to introduce an extra item for the default sort, then I think it should be specified in the marker schema: (maybe) sortProperty would designate which property inside the payload would be used to sort.

Thoughts?

Something like this would be great. And it can still default to name if not provided to maintain the current behaviour.