Give stylo rust code the ability to push and pop profiler label frames

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P3
normal
RESOLVED FIXED
8 months ago
Last month

People

(Reporter: mstange, Assigned: heycam)

Tracking

(Blocks 1 bug)

Trunk
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 wontfix, firefox68 fixed)

Details

Attachments

(1 attachment)

Bug 1500692 is adding subcategory support for profiler label frames, but those currently can't be used from rust code, which is unfortunate, see https://phabricator.services.mozilla.com/D11341#inline-48093 :

> Adding separate labels for property cascading / selector matching / style
> invalidation would be useful, though those are on the rust side, so not sure
> how easy to add they are. Is there any plan to add profiler support to rust
> code?

This bug is about adding this capability.

I'm not sure how it should be done. Should we expose the full category enum to rust? How should we set up the bindings to do the actual pushing and popping of the profiler labels? Should those be callback functions which are registered dynamically by Gecko during Stylo initialization?
Bindgen already understands enums and such for a lot of Gecko code, and runs after the export phase, so getting the enums there may not be the biggest issue...

If adding some FFI functions for the profiler to push and pop is not too hard, I'm happy to take a look and hook the bindings up.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
> If adding some FFI functions for the profiler to push and pop is not too
> hard, I'm happy to take a look and hook the bindings up.

Here's what Gecko's RAII object for profiler labels does: https://searchfox.org/mozilla-central/rev/17f55aee76b7c4610a974cffd3453454e0c8de7b/tools/profiler/public/GeckoProfiler.h#795-807,822-855
Should be simple enough to wrap in binding functions.
Over to the queue :)
Flags: needinfo?(emilio)
Priority: -- → P3
Depends on: 1500692

I have a patch for this. The one concern I have is that this does the profiler label push/pop for every Rayon work item. I haven't yet measured to see whether that's a lot of overhead. The alternative might be to do the push/pop down inside rayon::registry::wait_until_cold, but that's a bit less general as we really do have different kinds of work happening on the style threads -- style computation or CSS parsing. Though which category it is should be constant for a given rayon::Scope.

It's around 280 ns of overhead when styling is in full flight, which should be acceptable.

Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23396874909f
Add Gecko profiler labels for when the style threads are doing work. r=emilio
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Blocks: 1551761
Flags: needinfo?(emilio)
You need to log in before you can comment on or make changes to this bug.