-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
Report JavaScript thread #49395
Open
hoxyq
wants to merge
6
commits into
facebook:main
Choose a base branch
from
hoxyq:export-D69530984
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Report JavaScript thread #49395
+594
−43
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This pull request was exported from Phabricator. Differential Revision: D69530984 |
Summary: # Changelog: [Internal] Added OS-agnostic module that will implement 2 basic capabilities: - Getting current process id - Getting current thread id This information will later be used in `PerformanceTracer`, which is trace-engine for recording profiles that can be inspected in Chrome DevTools. Differential Revision: D69316093
Summary: # Changelog: [Internal] Use newly added module to get real OS-level process and thread ids. We will keep process id on an instance, because I don't think it could change over time? For thread id, we will get it at the time of event registration. Differential Revision: D69316094
Summary: Pull Request resolved: facebook#49083 # Changelog: [Internal] > NOTE: Some CI jobs are expected to fail, because changes in Hermes D67353585 should be landed first, and then grafted to Static Hermes. Added 2 new public methods to `PerformanceTracer` instance for registering metadata Trace Events for processes and threads. Differential Revision: D68439733 Reviewed By: huntie
Summary: # Changelog: [Internal] When we start tracing, we will record metadata event for current process. We will base it off the assumption that the thread where we handle `Tracing.start` event is the main one. We can't register the main thread yet: 1. We can't assume that CDP messages are handled on it - this is not the case for Windows and in a long term Inspector will be moved to its own thread 2. We don't have any similar mechanism as Runtime Executor, but for Main thread. This will require Host providing an executor for it. I've validated at the top of the stack that not registering the main thread should not affect how trace is displayed in CDT. Differential Revision: D69465926
Summary: Pull Request resolved: facebook#48836 # Changelog: [Internal] When `Tracing.start` CDP method is received, here is what happens next: - `TracingAgent`, which intercepts the event will propagate this status to `Instance` by calling `startTracing()` on `InstanceAgent`. - `InstanceAgent` will propagate it to `Runtime` entities. The only difference, there is no concept of tracing for Runtime, it will only have an API for starting sampling profiler. When `Tracing.end` CDP method is received, it is propagated in the same order. There is also `collect*()` methods for collecting profiles. This has multiple benefits: - We can control when `Runtime` or `Instance` are recreated and can ask them to start recording trace profiles right away. This may be required when we would add support for Reload and Profile from Performance panel. - We might leverage this setup in the future, once we add instrumentation for Network panel. `InstanceAgent` will get notified when tracing started and will correspondingly notify other part of the infrastructure that will be responsible for recording network calls. - We remain being fully agnostic to the actual `Runtime`, see corresponding `RuntimeTargetDelegate` for `V8`. We don't have the capacity for implementing and maintaining sampling profiler capabilities for it, but this approach unblocks it from technical perspective, if someone would want to invest into this. `InstanceProfile` is a superset of `RuntimeSamplingProfile`. In the future, `InstanceProfile` may also include things like `NetworkProfile`, or similar stuff. The definition for `RuntimeSamplingProfile` will be added in D68414421 and relies on the availability of new API for sampling profiler in Hermes. Differential Revision: D68327630 Reviewed By: huntie
Summary: # Changelog: [Internal] Adding a new method to `RuntimeTarget` that will register it for Tracing. In our case, it will schedule a callback on JS executor that will register JavaScript thread with `PerformanceTracer`. Reviewed By: huntie Differential Revision: D69530984
This pull request was exported from Phabricator. Differential Revision: D69530984 |
a94a3e4
to
d05ea87
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
fb-exported
p: Facebook
Partner: Facebook
Partner
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Changelog: [Internal]
Now that we keep reference to runtime executor, we can use it to report the JavaScript thread id to
PerformanceTracer
and register Instance for profiling.Differential Revision: D69530984