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

debug: consider calling ListPackageVars from variable request instead of scopes request #139

Open
ramya-rao-a opened this issue May 29, 2020 · 2 comments
Labels
debug/variables issues related to variables inspection/presentation Debug Issues related to the debugging functionality of the extension. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@ramya-rao-a
Copy link
Contributor

As being discussed in microsoft/vscode-go#2210, the feature of showing global variables when debugging requires calling ListPackageVars which is expensive in many cases. @segevfiner has a suggestion that we can call ListPackageVars just from the variable request instead of being called from scopes request.

I havent investigated whether this is possible and if so, whether it makes any difference to perf. This issue is to do the investigation and any follow up required

cc @polinasok, @quoctruong

@stamblerre stamblerre added the Debug Issues related to the debugging functionality of the extension. label May 29, 2020
@stamblerre stamblerre changed the title Consider calling ListPackageVars from variable request instead of scopes request debug: consider calling ListPackageVars from variable request instead of scopes request Jun 3, 2020
@stamblerre stamblerre added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 3, 2020
@gopherbot gopherbot added this to the Untriaged milestone Apr 8, 2021
@hyangah hyangah modified the milestones: Untriaged, Backlog Apr 14, 2021
@hyangah
Copy link
Contributor

hyangah commented Jan 5, 2022

@suzmue @polinasok Is this still relevant to dlv dap?

@polinasok
Copy link
Contributor

polinasok commented Jan 6, 2022

Per spec, variables request retrieves the children. That's when we preload and analyze the type of each child, so we can create a record and a reference for compound types (otherwise its a leaf with reference 0). This also allows us to display an informative value string with children details even before the user expands. Since a scope essentially acts like an outer variable, it is handled the same way and things are indeed loaded before the expand button is actually clicked, even though we don't put those details in the top-level string. There is a TODO in the delve code to reconsider this and delay loading:
https://github.com/go-delve/delve/blob/5842c1fe9e4009ebf020839fe4901a24c930ecb3/service/dap/server.go#L2034-L2041.
I think the cost aspect should be less relevant now because we no longer pay the price for sending the retrieved variables data over the network preemptively, but either way profiling is our friend in such cases to avoid premature optimizations at the expense of special cases and logic surgeries. At the same time this could be a good time to introduce another layer to support all Globals, not just main, which could help delay loading until the user actual clicks on Globals:

> Globals # use expansion to call s.debugger.CurrentPackage to load all
   > package main # use expansion to filter the cached values and send back just for the package
   > package foo 

@polinasok polinasok added the debug/variables issues related to variables inspection/presentation label Apr 8, 2022
@hyangah hyangah removed the DA: DlvDAP label Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug/variables issues related to variables inspection/presentation Debug Issues related to the debugging functionality of the extension. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

6 participants