Closed
Bug 1505908
Opened 2 years ago
Closed 2 years ago
Give stylo rust code the ability to push and pop profiler label frames
Categories
(Core :: CSS Parsing and Computation, enhancement, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla68
People
(Reporter: mstange, Assigned: heycam)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
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•2 years ago
|
||
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.
Reporter | ||
Comment 2•2 years ago
|
||
(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.
Assignee | ||
Updated•2 years ago
|
Priority: -- → P3
Assignee | ||
Comment 4•2 years ago
|
||
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.
Assignee | ||
Comment 5•2 years ago
|
||
It's around 280 ns of overhead when styling is in full flight, which should be acceptable.
Assignee | ||
Comment 6•2 years ago
|
||
Assignee | ||
Comment 7•2 years ago
|
||
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
Comment 9•2 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox68:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → cam
Updated•2 years ago
|
Flags: needinfo?(emilio)
Updated•2 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•