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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Markus Stange [:mstange]]

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?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

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.

Comment 2

[Markus Stange [:mstange]]

(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.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Over to the queue :)

Comment 4

[Cameron McCormack (:heycam) (away Jan 26 - Feb 1)]

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.

Comment 5

[Cameron McCormack (:heycam) (away Jan 26 - Feb 1)]

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

Comment 6

[Cameron McCormack (:heycam) (away Jan 26 - Feb 1)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=713aa8a3469df9503a36fc2d961ff82428fe77e3

Comment 7

[Cameron McCormack (:heycam) (away Jan 26 - Feb 1)]

Attachment: Bug 1505908 - Add Gecko profiler labels for when the style threads are doing work.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23396874909f
Add Gecko profiler labels for when the style threads are doing work. r=emilio

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/23396874909f