Use fast in-proc heuristic to implement HasChangesAsync#61690
Use fast in-proc heuristic to implement HasChangesAsync#61690tmat merged 9 commits intodotnet:mainfrom
Conversation
src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs
Show resolved
Hide resolved
src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We do not necessarily need to apply changes made during break state before the break state ends
This is somewhat scary sounding, but also somewhat interesting. I like the idea that some rude edits become acceptable if applied outside of break state. Should we update the diagnostic message to indicate that? eg "Updates around active statements cannot be made while program execution is paused. Use Apply Changes during program execution to try again"
Alternatively, should we have a list of rude edits that this applies to, and try to automatically apply them after break state has ended, at idle or after a delay? Or is that a job for the debugger?
src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs
Show resolved
Hide resolved
These rude edits mention "active statement" in the message (e.g. "can't edit around an active statement"). This implies that the presence of the active statement is the issue. It might not be that obvious and we could change the message a bit to make it more obvious. Feel free to file an issue to follow up.
Note that we don't pick which changes to apply and which not. All changes are either applied or not. We just choose to not apply in certain, rare cases. I don't think we should automatically apply any changes while the app is running without the customer explicitly asking us to do so. That might result in surprising behavior. Applying outside of break session (Hot Reload) has different semantics than applying when the app is stopped. Hot Reload changes are applied only to the next invocation of the method, not the current frame like EnC. |
EditAndContinueLanguageService.HasChangesAsyncis called on UI thread when stepping to determine whether any changes have been made that the debugger needs to apply before resuming the execution of the debuggee. Previously we called to the EnC OOP service to determine the changes precisely. This determination may involve running code generators and checking document content against PDB checksums, which may cause long UI delays.@isadorasophia pointed out that precision is not required here and would only marginally degrade the user experience in rare cases. If any changes are missed during stepping (in rare cases) and the user observes the difference they can apply the changes explicitly via Apply Changes command. Skipping a change application may lead to interesting effects in certain corner cases, but it appears to be sound.
EnC service creates a series of
EditSessioninstances. Each instance represents state of the session in between points that change some aspect of the session (inputs to the EnC analysis) . The first instance is created when the debugging/Hot Reload session starts (F5/Ctrl+F5).A new session is created, replacing the previous one (see
DebuggingSession.RestartEditSession) at the following events:Prior to this change events [1] and [3] have been implicitly coupled. The debugger always applied changes before exiting the break state. However, we can also see them as independent. We do not necessarily need to apply changes made during break state before the break state ends. We can skip applying the changes at that point and apply them later - either while the app is running (if the user explicitly triggers Apply Changes) or at the next break state (automatically, if we detect them at that time).
All delta analysis continues to use the latest committed solution without regard of the break state. The only impact of entering and exiting break state on the EnC analysis is consideration of active statements, which are only available while at break state. It may happen that the changes were considered rude during the break state (due to presence of an active statement) while they might be allowed while the application is running. The background EnC analysis will always report the correct rude edits (squiggles) at that point. It may happen, if we skip applying a change, that the squiggle is shown while at break state, disappears when break state is exited and appears again when the app is stopped at the next break state.
Now that we determined that skipping change application during break state is sound, we can perform much faster check (heuristic) that only inspect readily available in-proc solution state. Ideally, we would avoid using solution snapshot entirely for this implementation, but that turns out to be tricky. We could, for example, track changes to open files that belong to Roslyn and ignore changes that were made outside of VS. However, determining whether a file belongs to Roslyn (i.e. can affect compilation) is not straightforward as it depends on data coming from the project system (especially for additional files, which can have arbitrary extension). In future we might want to investigate this approach further: #61692.
Fixes AB#1540395
Fixes AB#1370311
Fixes AB#1240764
Fixes #55625