Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-135953: Add Gecko reporter to sampling profiler#139364
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ivonastojanovic commented Sep 26, 2025 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
pablogsal commented Sep 26, 2025
CC @lkollar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Gecko reporter functionality to the sampling profiler, enabling export of profiling data in Gecko Profile format for web-based visualization tools. The implementation includes a new GeckoCollector class and supporting data structures.
- Adds new gecko_format.py module with dataclasses and builder for Gecko profile format
- Implements GeckoCollector class in stack_collector.py that exports profiling data as JSON
- Updates sample.py to support --gecko command line option and integrate the new collector
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Lib/profiling/sampling/gecko_format.py | New module defining Gecko profile format dataclasses and builder |
| Lib/profiling/sampling/stack_collector.py | Adds GeckoCollector class and category constants |
| Lib/profiling/sampling/sample.py | Integrates Gecko collector option and updates CLI help text |
Comments suppressed due to low confidence (1)
Lib/profiling/sampling/stack_collector.py:1
- GeckoCollector is instantiated without the skip_idle parameter that other collectors receive. Consider whether skip_idle functionality should be supported for consistency with other collectors.
import base64 Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
pablogsal commented Sep 27, 2025
There are several problems here:
|
pablogsal commented Sep 27, 2025
This also will need some tests |
cf36ebb to cc770b6Comparepablogsal commented Sep 27, 2025
@ivonastojanovic I have pushed a draft version of a simpler collector that uses raw dictionaries. This can be processed by the firefox profiler, doesn't blow up the memory in my experiments and also is relatively faster. It needs some polishing but should cover most problems. |
cc770b6 to fe9adcdCompare1b3b603 to 352ea21CompareUh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Pablo Galindo Salgado <pablogsal@gmail.com>
75b1afe into python:mainUh oh!
There was an error while loading. Please reload this page.
pablogsal commented Oct 1, 2025
Great work @ivonastojanovic ! |
bedevere-bot commented Oct 1, 2025
|
pablogsal commented Oct 1, 2025
This looks unrelated to this PR as this PR only adds |
Issue: #135953