-
Notifications
You must be signed in to change notification settings - Fork 223
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
Lazily Evaluate Variables/threads/stacktrace for better stepping performance #2189
Comments
So my testing was correct and I was able to drastically speed up the UI responsiveness by returning an initial stack frame based on the invocation info, and then the subsequent DelayedStackTraceLoad awaits a task that returns the rest from the Capture.mp4I will have to unwind some of the internal relationship between stacktrace and their variables, because per the DAP spec variables can also be lazy resolved, but PSES currently requires vars to be fully resolved to deliver a stacktrace object, so I'll more loosely couple those relationships and then put the PR up. |
Looks like this is confirmed by microsoft/debug-adapter-protocol#344 and microsoft/debug-adapter-protocol#79 |
This is working very well but the catch is that I can't overwrite stacktraces (asked for DAP to clarify), so I create a special Capture.mp4@andyleejordan if you're OK with this approach I'll rewire up the variable handling and then provide a PR. |
Out of curiosity, I also looked into how I understand that the reasoning of the "glorious hack" as @andyleejordan put it was to unify both remote and local runspaces to serialize the info in a single channel, but if we are on a local runspace, using the API doesn't require it go thru the pipeline and completes pretty much immediately. runspaceContext.CurrentRunspace.Runspace.Debugger.GetCallStack()
.Select(StackFrameDetails.Create) So I am going to do a version that utilizes this if it is a local runspace or otherwise our call stack is accessible via the debugger API rather than fetching it inline at the debug prompt. EDIT: Yep in this version, evaluating the callstack only takes 3ms for a relatively basic callstack, and thats with fetching the callstack within the handler, if we pre-fetch it at the debugStop event, probably even faster. |
This is amazing work Justin. That "glorious hack" was Rob's, for sure. I'm personally of the opinion that supporting remote debugging through nested |
@andyleejordan correct, future PRs beyond #2190 will improve that, I just didn't want to drop a giant PR on you that changes everything and takes forever to review. |
PowerShellEditorServices/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs
Lines 985 to 986 in bbf627b
This line "blocks" stepping for about half a second, and I don't currently see any reason why it has to. We should return to the user as fast as possible that we have reached our breakpoint, and do this heavy processing at the point the DAP makes the
stackTrace
,scopes
, andvariables
requests, because those are non-blocking to the UIEDIT: Based on DAP testing, currently after a stop, vscode requests
threads
andstacktrace
before the UI updates with the new breakpoint, but I think if we implementsupportsDelayedStackTraceLoading
capabilities, we can quickly return a first stack trace, and since threads is basically a dummy since we don't currently support multi-thread stacktraces, we can probably make some responsiveness improvements here.The text was updated successfully, but these errors were encountered: