PowerShell/PowerShellEditorServices

Lazily Evaluate Variables/threads/stacktrace for better stepping performance

Opened this issue · 8 comments

await FetchStackFramesAndVariablesAsync(noScriptName ? localScriptPath : null).ConfigureAwait(false);

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, and variables requests, because those are non-blocking to the UI

EDIT: Based on DAP testing, currently after a stop, vscode requests threads and stacktrace before the UI updates with the new breakpoint, but I think if we implement supportsDelayedStackTraceLoading 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.

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 Get-PSCallStack invocation.

Capture.mp4

I 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.

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 <breakpoint> frame using similar invocation info to what @SeeminglyScience was doing in augmenting the first stack trace frame. I don't think this is too distracting or inaccurate, and it makes the stepping experience much better.

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 Get-PSCallStack works, and then found the API it uses is public.

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.
image

I was also able to use the Position info to make the call stack more specific rather than simply at the line level
image
image

There is a label presentation style for stack frames specifically for this kind of artificial presentation, that looks much better:
image

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 PSSessions is not the highest priority especially if it gets in the way of making local debugging a superb experience, because we can (and should) just be able to use that across VS Code's native remote debugging (via SSH etc.).

@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.