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

Enable taking stack snapshots of other threads: The Linux/Android edition #22203

Closed
gterzian opened this issue Nov 15, 2018 · 2 comments
Closed

Enable taking stack snapshots of other threads: The Linux/Android edition #22203

gterzian opened this issue Nov 15, 2018 · 2 comments

Comments

@gterzian
Copy link
Member

@gterzian gterzian commented Nov 15, 2018

Follow up on #16740

Code in components/background_hang_monitor/sampler.rs(yet to be merged from #21673)

The suspending/resuming of a thread is (perhaps only partly) implemented, and it is mainly accessing the instruction and frame pointers in the registers, as well as perhaps the stack walking, that needs to be (re-)implemented for Linux/Android.

I think target architectures for Linux would be x86_64, and for Android it would be x86 for the emulator, and arm for the devices.

It appears that libc would be the appropriate tool in both cases.

See also the Gecko implementation at https://dxr.mozilla.org/mozilla-central/rev/b0b856065d5b7ad2996f707e6e797d0d72afd803/tools/profiler/core/platform-linux-android.cpp#85

@nikhilm
Copy link

@nikhilm nikhilm commented Nov 21, 2018

FWIW, I have a work in progress library (i only made it publicly viewable so i could post this, heh! still needs documentation and so on) for a sampling profiler. https://bitbucket.org/nikhilm/vignette.
It already implements Linux thread suspension and resume, and was actually inspired by the gecko implementation. I work on this really on and off, so I won't be able to fix this issue itself, but feel free to copy the code from https://bitbucket.org/nikhilm/vignette/src/master/src/lib_linux.rs

@gterzian
Copy link
Member Author

@gterzian gterzian commented Nov 22, 2018

@nikhilm thanks!

I'll restructure the Mac-Os one to use a similar Sampler abstractions and and then we can use your implementation for Linux/Android.

I assume your PosixSemaphore beats what I was trying to do with https://github.com/servo/servo/pull/21673/files#diff-b842b7c119b3715c2ee8e59aa3df11a2R32

Also, your implementation of Sample.collect fills in a gap in the current linux work in progress(I only go the equivalent to work on Mac).

For the yet to be implemented resolving of the frames, we can use backtrace::resolve which will work across platforms...

I'll focus on re-structuring the Mac part, as well as the overall structure, so that it will "fit" better with this, I might try the actual Linux implementation after that, and if anyone else is interested, just let me know...

@gterzian gterzian mentioned this issue Nov 22, 2018
0 of 5 tasks complete
@gterzian gterzian mentioned this issue Dec 3, 2018
4 of 5 tasks complete
bors-servo added a commit that referenced this issue Mar 26, 2019
Add linux sampler

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #22203 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22355)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 28, 2019
Add linux sampler

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #22203 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22355)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 29, 2019
Add linux sampler

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22203 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22355)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 30, 2019
Add linux sampler

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22203 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22355)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 30, 2019
Add linux sampler

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22203 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22355)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.