-
-
Notifications
You must be signed in to change notification settings - Fork 33k
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
Conversation
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.
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 |
There are several problems here:
|
This also will need some tests |
cf36ebb
to
cc770b6
Compare
@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
fe9adcd
Compare
1b3b603
to
352ea21
Compare
Signed-off-by: Pablo Galindo Salgado <[email protected]>
44ec90d
to
35c4e5c
Compare
Great work @ivonastojanovic ! |
|
This looks unrelated to this PR as this PR only adds |
Issue: #135953