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 line number to CPU stack frame URLs #47215

Closed
kenzieschmoll opened this issue Sep 14, 2021 · 4 comments
Closed

Add line number to CPU stack frame URLs #47215

kenzieschmoll opened this issue Sep 14, 2021 · 4 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@kenzieschmoll
Copy link
Contributor

Right now, the json payload for a single CPU stack frame returns a field resolvedUrl with the file path for the stack frame. Can we add the line number to this data? @bkonyi

@lrhn lrhn added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Sep 15, 2021
@bkonyi
Copy link
Contributor

bkonyi commented Sep 16, 2021

Do you mean as an extra property, or appended to the end of the URI? I think the latter could potentially cause issues. Also, you should have source position information available via the ProfileFunction's function property.

@kenzieschmoll
Copy link
Contributor Author

kenzieschmoll commented Sep 23, 2021

From ProfileFunction, we can eventually get the ScriptRef for a CPU stack frame (function.function.location.script). However, the only property on ScriptRef is the uri. This means to get the line number for this ScriptRef, we'd have to fetch the script from the vm service, which will add a significant amount of processing overhead (I tried quick implementation of this and it cripples performance). Could we instead just add an extra property to ProfileFunction that has the sourceLineOffset?

copybara-service bot pushed a commit that referenced this issue Jan 12, 2022
Removes the need for requesting a full Script object, which can be
extremely large when including source code. This change will have a
relatively small impact on response sizes.

Related issues: #47215, flutter/devtools#3382

TEST=pkg/vm_service tests updated

Change-Id: I27999c4b1da65d4f0c643fa8db1a019c0fd1d689
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/227640
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
@bkonyi
Copy link
Contributor

bkonyi commented Jan 13, 2022

Landed 2db8f37 with support for line/column information in SourceLocation responses. Still need to publish the vm_service changes.

@bkonyi
Copy link
Contributor

bkonyi commented Feb 15, 2022

vm_service changes are published and will be rolled into the Flutter framework shortly.

@bkonyi bkonyi closed this as completed Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

3 participants