We have some calls to the Gecko profiler in classes such as nsStyleSet and ElementRestyler (which add labels) that won't be run when stylo is enabled. We need to do something equivalent from the Rust side when we are using stylo.
We also might need to register the style worker threads with the profiler.
I would like to investigate this. Chris, do you know if anybody else is working on this?
(In reply to Nathan Froyd [:froydnj] from comment #2) > I would like to investigate this. Chris, do you know if anybody else is > working on this? Thanks! No one is looking at the profiler yet, but profiler support will be important once we start getting Hasal performance reports.
Opened https://github.com/nikomatsakis/rayon/issues/225 to discuss adding the necessary support to rayon.
Just FYI that I'm approaching the point where I'd like to start profiling the parallel traversal. Still probably a few days off but would be nice to get the threads registered.
Niko, do you have an ETA of when rayon 0.7 will be released? You just merged Nathan's rayon thread startup/shutdown changes  that necessary to add Rust support to the Gecko Profiler. If there are other post-v0.6 rayon changes that still need stabilization before you can release 0.7, would you consider a cutting a 0.7 release that is just 0.6 plus Nathan's thread startup/shutdown changes? This will be a big help for the Stylo team's profiling efforts for Quantum.  https://github.com/nikomatsakis/rayon/issues/225
Created attachment 8855339 [details] [diff] [review] add servo bindings for profiler thread registration We need these to be able to register parallel traversal threads with the profiler. Getting the Gecko side stuff ready to go so perhaps the servo-side stuff will be easier.
Niko just released rayon 0.7: https://github.com/nikomatsakis/rayon/releases/tag/v0.7.0
Comment on attachment 8855339 [details] [diff] [review] add servo bindings for profiler thread registration Review of attachment 8855339 [details] [diff] [review]: ----------------------------------------------------------------- I'm not familiar with profiler at all, but this patch looks reasonable and simple enough, so rs=me.
It seems to me there is a mutex in profiler_register_thread unconditionally hold. Would that be a problem given we may spawn several threads at once?
(In reply to Xidorn Quan [:xidorn] UTC+10 (less responsive 15/Apr-3/May) from comment #10) > It seems to me there is a mutex in profiler_register_thread unconditionally > hold. Would that be a problem given we may spawn several threads at once? We can only register one thread at a time; it's a bit problematic in that some threads might start work in the thread pool before others have even spawned, but I don't think it will be a problem in practice? We could add a barrier to ensure all the threads have started and registered before they start work, though?
Oh, hmm, so we have a thread pool for that, I guess that's fine then. I thought that we spawn threads for every tree traversal, where the additional hook could take time. But if we reuse threads a lot, that wouldn't be a problem then.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/d24b3983c4c2 add servo bindings for profiler thread registration; r=xidorn
sorry had to back this out causing conflicts merging inbound to m-c like for warning: conflicts while merging layout/style/ServoBindings.cpp
Backout by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1e5d7a2c00b6 Backed out changeset d24b3983c4c2 for causing merge conflict with m-c
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/3146ac58280a add servo bindings for profiler thread registration; r=xidorn
Is there more to do here?
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #18) > Is there more to do here? There are two things left to do on the servo side: * Integrate rayon 0.7: https://github.com/servo/servo/pull/16303 This is blocked by Mac test timeouts. My Mac seems incapable of running servo atm, so I have a new machine ordered, but if you knew of someone who could debug these, that would be great! * Add patches to servo that use the new rayon APIs. I have this patch written, it's just waiting on rayon.
rayon 0.7 has been merged to servo master and mozilla-autoland: https://github.com/servo/servo/commit/fc3a7d03f2b0c5292e7414e7555fb75ae0b1d663 https://hg.mozilla.org/integration/autoland/rev/62536f85ab9c226b5997cb07e7f10e164b5db8f1
https://github.com/servo/servo/pull/16786 has landed on autoland, so stylo threads should be able to be seen by the Gecko Profiler. You'll have to manually add them to the list of threads the Gecko Profiler is following. I'll send a message to stylo-team after autoland merges to m-c (probably tomorrow morning my time) with an announcement and what you need to do.
:froydnj could you or someone share a few profiles from perf.html of stylo here so that I can take a look at what those profiles look like? It'll be helpful for working on the performance client. We have a few open issues I'd like to include them on.
(In reply to Greg Tatum [:gregtatum] [@gregtatum] from comment #22) > :froydnj could you or someone share a few profiles from perf.html of stylo > here so that I can take a look at what those profiles look like? It'll be > helpful for working on the performance client. We have a few open issues I'd > like to include them on. You just want a profile with the Stylo threads active (and ideally doing something)? I'm curious what you expect to get out of such profiles that a semi-random profile wouldn't already give you?
I like working from real data when working with features. I have a request to support merging of threads in perf.html. It would be nice when developing a feature to see if the resulting functionality was sensible with real data. https://github.com/devtools-html/perf.html/issues/88
Here's a profile I took of : https://perfht.ml/2qrbely Notice that filtering the content process main thread on Servo_TraverseSubtree will show you the segments where the main thread kicks over to the parallel workers.  https://en.wikipedia.org/wiki/Barack_Obama
And in particular, it sure would be nice to coalesce those worker threads!