Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a sampling profiler #23080

Merged
merged 3 commits into from Mar 26, 2019

Conversation

Projects
None yet
5 participants
@jdm
Copy link
Member

commented Mar 22, 2019

This uses the code already built for the background hang monitor and adds the ability to repeatedly sample all monitored threads. This sampling allows us to generate profiles that we can translate into the format used by https://perf-html.io/, allowing us to benefit from modern Gecko performance tooling.

You can run Servo with PROFILE_OUTPUT=foo.json and SAMPLING_RATE=50 (for example), otherwise these values will default to samples.json and 10ms, respectively. To activate the profiler, press cmd+p, and to stop profiling, press cmd+p again. This will the captured samples to be symbolicated, which will take a very long time, and eventually there will be a new JSON profile in the output location.

To create a profile for use by Gecko's tools, run python etc/profilicate.py path/to/profile.json >gecko_profile.json, and load gecko_profile.json in the https://perf-html.io/ to see something like this;


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #13103
  • These changes do not require tests because way too many pieces to automate

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented Mar 22, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/constellation/pipeline.rs, components/constellation/constellation.rs
  • @cbrewster: components/constellation/pipeline.rs, components/constellation/constellation.rs
  • @paulrouget: components/constellation/pipeline.rs, components/constellation/constellation.rs, components/compositing/windowing.rs, ports/servo/browser.rs, components/servo/lib.rs
@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 22, 2019

@gterzian Since a lot of this work was just plugging together the existing pieces from the background hang monitor, would you like to review this?

@jdm jdm force-pushed the jdm:sampling-profiler branch 3 times, most recently from f7f4bc9 to 27361c5 Mar 22, 2019

@jdm jdm force-pushed the jdm:sampling-profiler branch from 27361c5 to 1a2db82 Mar 22, 2019

@gterzian

This comment has been minimized.

Copy link
Collaborator

commented Mar 24, 2019

@jdm Ok, I will take a look...

@gterzian
Copy link
Collaborator

left a comment

I have a slight re-design proposal for the constellation -> bhg communication for consideration.

It's a bit complicated and I think it might be worth it since it will enable toggling the sampling also in multiprocess mode:

  1. Replace the new toggle with a communication mechanism between the constellation and the bhg worker, to be setup inside HangMonitorRegister::init(which in the case of multiprocess servo will be called from inside the content-process).
  2. Have the HangMonitorRegister create an ipc channel for a new SamplerControlMsg message type, and then send the the sender to the constellation(using the IpcSender<HangMonitorAlert> that is available), and adding the receiver to BackgroundHangMonitorWorker::new.
  3. Have BackgroundHangMonitorWorker::run include the ipc receiver in a select via ROUTER.route_ipc_receiver_to_new_crossbeam_receiver.
  4. Have the constellation store a Vec<IpcSender<SamplerControlMsg>> containing one sender per content-process, or just one if in single-process mode.
  5. Have the embedder toggle sampling via messaging to the constellation, which will then send a SamplerControlMsg::StartSampling or SamplerControlMsg::StopSampling message to the bhg of each content-process.
  6. handle_sampling would then become just another message handler in the bhg event-loop, and SamplerState just another member on the BackgroundHangMonitorWorker.
while monitor.run() {
// Monitoring until all senders have been dropped...
}
});
Box::new(HangMonitorRegister { sender })
}

pub fn toggle(rate: Duration) {

This comment has been minimized.

Copy link
@gterzian

gterzian Mar 24, 2019

Collaborator

I think there are two issues with this setup:

  1. I think it's not going to work in multiprocess mode(since toggle is called from the "main" process where the embedder is running).
  2. When you call toggle one cannot be sure that the bhg is running, so toggle could return successfully but not result in anything changing.
  3. In practice there will only ever be one bhg per process, so the mutex around SamplerState is a pity.

@jdm jdm force-pushed the jdm:sampling-profiler branch from db896f2 to f5bf232 Mar 25, 2019

@jdm jdm requested a review from gterzian Mar 25, 2019

@gterzian
Copy link
Collaborator

left a comment

Looks good! It is indeed a better idea to pass the control chan to the constellation when the pipeline is created, as opposed to send it in a message like I initially suggested...

There are just a few println that I guess you still plan to remove or change into debug(and the tests need an update)?

Otherwise just one comment...

@@ -1195,6 +1217,16 @@ where
self.forward_event(destination_pipeline_id, event);
},
FromCompositorMsg::SetCursor(cursor) => self.handle_set_cursor_msg(cursor),
FromCompositorMsg::EnableProfiler(rate) => {
for chan in &self.sampling_profiler_control {
let _ = chan.send(SamplerControlMsg::Enable(rate));

This comment has been minimized.

Copy link
@gterzian

gterzian Mar 26, 2019

Collaborator

Perhaps expect or warn on failure for both sends to the monitor? The constellation keeps two channels to the monitor so something is wrong if the monitor isn't receiving messages anymore...

bors-servo added a commit that referenced this pull request Mar 26, 2019

Auto merge of #23095 - jdm:mozjsprofiling, r=ferjm
Use mozjs profiling feature when building with frame pointer enabled

This makes it possible to get more meaningful profiles with #23080.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #23081
- [x] There are no tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23095)
<!-- Reviewable:end -->

bors-servo added a commit that referenced this pull request Mar 26, 2019

Auto merge of #23095 - jdm:mozjsprofiling, r=ferjm
Use mozjs profiling feature when building with frame pointer enabled

This makes it possible to get more meaningful profiles with #23080.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #23081
- [x] There are no tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23095)
<!-- Reviewable:end -->

bors-servo added a commit that referenced this pull request Mar 26, 2019

Auto merge of #23095 - jdm:mozjsprofiling, r=jdm
Use mozjs profiling feature when building with frame pointer enabled

This makes it possible to get more meaningful profiles with #23080.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #23081
- [x] There are no tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23095)
<!-- Reviewable:end -->
@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

Updated the tests. I intend to keep the printlns, because there is no feedback on what's happening otherwise and resolving can take a really long time, and quitting while it's resolving means the profile is lost.

@jdm jdm force-pushed the jdm:sampling-profiler branch from 4ab0960 to 946fdc8 Mar 26, 2019

@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

@bors-servo r=gterzian

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

📌 Commit 946fdc8 has been approved by gterzian

@highfive highfive assigned gterzian and unassigned asajeffrey Mar 26, 2019

@jdm jdm force-pushed the jdm:sampling-profiler branch from 946fdc8 to 7befe85 Mar 26, 2019

@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

@bors-servo r=gterzian

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

📌 Commit 7befe85 has been approved by gterzian

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

⌛️ Testing commit 7befe85 with merge c7be968...

bors-servo added a commit that referenced this pull request Mar 26, 2019

Auto merge of #23080 - jdm:sampling-profiler, r=gterzian
Add a sampling profiler

This uses the code already built for the background hang monitor and adds the ability to repeatedly sample all monitored threads. This sampling allows us to generate profiles that we can translate into the format used by https://perf-html.io/, allowing us to benefit from modern Gecko performance tooling.

You can run Servo with `PROFILE_OUTPUT=foo.json` and `SAMPLING_RATE=50` (for example), otherwise these values will default to `samples.json` and 10ms, respectively. To activate the profiler, press cmd+p, and to stop profiling, press cmd+p again. This will the captured samples to be symbolicated, which will take a very long time, and eventually there will be a new JSON profile in the output location.

To create a profile for use by Gecko's tools, run `python etc/profilicate.py path/to/profile.json >gecko_profile.json`, and load `gecko_profile.json` in the https://perf-html.io/ to see something like [this](https://profiler.firefox.com/public/8137e2b11fbb92afb80090bc534fd83015c87ee6/calltree/?globalTrackOrder=0-1&thread=1&v=3);

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13103
- [x] These changes do not require tests because way too many pieces to automate

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23080)
<!-- Reviewable:end -->

@jdm jdm force-pushed the jdm:sampling-profiler branch from 7befe85 to aee2974 Mar 26, 2019

@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

@bors-servo r=gterzian

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

📌 Commit aee2974 has been approved by gterzian

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

⌛️ Testing commit aee2974 with merge 2e0191b...

bors-servo added a commit that referenced this pull request Mar 26, 2019

Auto merge of #23080 - jdm:sampling-profiler, r=gterzian
Add a sampling profiler

This uses the code already built for the background hang monitor and adds the ability to repeatedly sample all monitored threads. This sampling allows us to generate profiles that we can translate into the format used by https://perf-html.io/, allowing us to benefit from modern Gecko performance tooling.

You can run Servo with `PROFILE_OUTPUT=foo.json` and `SAMPLING_RATE=50` (for example), otherwise these values will default to `samples.json` and 10ms, respectively. To activate the profiler, press cmd+p, and to stop profiling, press cmd+p again. This will the captured samples to be symbolicated, which will take a very long time, and eventually there will be a new JSON profile in the output location.

To create a profile for use by Gecko's tools, run `python etc/profilicate.py path/to/profile.json >gecko_profile.json`, and load `gecko_profile.json` in the https://perf-html.io/ to see something like [this](https://profiler.firefox.com/public/8137e2b11fbb92afb80090bc534fd83015c87ee6/calltree/?globalTrackOrder=0-1&thread=1&v=3);

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13103
- [x] These changes do not require tests because way too many pieces to automate

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23080)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

@bors-servo bors-servo merged commit aee2974 into servo:master Mar 26, 2019

4 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

@bors-servo bors-servo referenced this pull request Mar 26, 2019

Merged

Add linux sampler #22355

4 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.