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

SCM Tab refresh on every keypress on editor #179441

Open
anandbibek opened this issue Apr 7, 2023 · 14 comments · Fixed by #188506
Open

SCM Tab refresh on every keypress on editor #179441

anandbibek opened this issue Apr 7, 2023 · 14 comments · Fixed by #188506
Assignees
Labels
feature-request Request for new features or functionality scm General SCM compound issues
Milestone

Comments

@anandbibek
Copy link

Type: Bug

The issue is reproducible when connected to remote env via SSH and there is an active SCM system recognised.

Every keypress on the editor shows the SCM tab refreshing for a split second. This also somehow makes the code completion and suggestion popups appear after that SCM refresh is done. This causes an overall delay in working with the editor and getting suggestions.

VS Code version: Code 1.77.1 (b7886d7, 2023-04-04T23:21:11.906Z)
OS version: Windows_NT x64 10.0.22000
Modes:
Sandboxed: No
Remote OS version: Linux x64 5.4.17-2136.317.5.3.el8uek.x86_64
Remote OS version: Linux x64 5.4.17-2136.317.5.3.el8uek.x86_64

System Info
Item Value
CPUs 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz (8 x 1805)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: disabled_off
Load (avg) undefined
Memory (System) 31.73GB (19.81GB free)
Process Argv
Screen Reader no
VM 0%
Item Value
Remote SSH: anaray-dev
OS Linux x64 5.4.17-2136.317.5.3.el8uek.x86_64
CPUs AMD EPYC 7J13 64-Core Processor (4 x 2445)
Memory (System) 31.08GB (20.67GB free)
VM 0%
Item Value
Remote SSH: anaray-dev
OS Linux x64 5.4.17-2136.317.5.3.el8uek.x86_64
CPUs AMD EPYC 7J13 64-Core Processor (4 x 2445)
Memory (System) 31.08GB (20.67GB free)
VM 0%
Extensions (37)
Extension Author (truncated) Version
Bookmarks ale 13.3.1
tsl-problem-matcher amo 0.6.2
chalice-icon-theme art 1.2.22
beardedicons Bea 1.10.0
LogFileHighlighter emi 2.16.0
intellij-idea-keybindings k-- 1.5.9
feather-vscode mel 1.0.1
fluent-icons mig 0.0.18
theme-monokai-pro-vscode mon 1.2.0
JetBrainsIcons Mos 1.0.5
remote-ssh ms- 0.98.0
remote-ssh-edit ms- 0.84.0
remote-explorer ms- 0.4.0
rose-pine mvl 2.8.0
back-n-forth nic 3.1.1
material-icon-theme PKi 4.25.0
material-product-icons PKi 1.5.0
vscode-icons vsc 12.2.0
wvsc-serendipity wic 0.105.0
material-theme zhu 3.15.8
ade-source-control Ana 0.0.36
java-project-stup Ana 4.0.3
vscode-eslint dba 2.4.0
svg joc 1.5.2
symbols mig 0.0.9
vscode-filesize mkx 3.1.0
vscode-dotnet-runtime ms- 1.6.0
java red 1.16.0
intellicode-api-usage-examples Vis 0.2.7
vscodeintellicode Vis 1.2.30
vscode-java-debug vsc 0.49.1
vscode-java-dependency vsc 0.21.2
vscode-java-pack vsc 0.25.10
vscode-java-test vsc 0.38.2
vscode-maven vsc 0.41.0
jar-viewer wma 1.2.0
change-case wma 1.0.0

(17 theme extensions excluded)

@anandbibek
Copy link
Author

This happens only when connected to remote dev environment and we have an active SCM plugin. I have tested with Git as well as my own custom SCM plugin. But I have not found my scm plugin code being called during that refresh icon popping up

Recording.2023-04-07.103317.mp4

@anandbibek
Copy link
Author

Recording.2023-04-07.103606.mp4

Showing the tab and progress indicator as well

@anandbibek
Copy link
Author

anandbibek commented Apr 7, 2023

I've narrowed it down to registration of quickDiffProvider. This function gets called on every keypress provideOriginalResource(uri: Uri)

As soon as I register it, this behavior starts happening. I won't assume this is a fault of my plugin, as it happens for git plugin as well.
And I feel like this was not happening maybe a few versions back..

@lszomoru
Copy link
Member

@anandbibek, apologies for not getting back to you on this until now. Are you able to reproduce this issue with the latest Stable release of VS Code? There have been couple of known issues related to the quick diff provider that have been fixed since you have filed this issue. Thank you!

@lszomoru lszomoru added the info-needed Issue requires more information from poster label May 17, 2023
@anandbibek
Copy link
Author

I do see this happening every now and then, but the impact seems to have reduced a bit. Sometimes feels much faster if I just reload the window. Let me re-evaluate this and get back with latest observation.

@anandbibek
Copy link
Author

Hi @lszomoru , I just debugged my extension right now. I still see the function public async provideOriginalResource(uri: Uri): Promise<Uri | undefined> from my quick diff provider is getting called on every keypress on the editor. Same thing with Git as well. And you can clearly see the busy indicator pop up on SCM tab/badge.

I don't think this is necessary. Maybe once every file save or maybe even less..

Problem is that every time it gets called, the autocomplete, suggestions, etc. and other editor functions get invoked only after that call returns. Introducing a clear delay in the interaction with editor.

@lszomoru
Copy link
Member

lszomoru commented Jun 6, 2023

@anandbibek, I would really like to get to the bottom of this so I was wondering if you could provide more information so that I can attempt to reproduce the issue. Looking at the list of extensions that you have installed, is it safe to assume that you are using Remote-SSH in order to connect to your remote machine? Are you able to reproduce the same behaviour with all extensions disabled except Remote-SSH? Thank you!

@anandbibek
Copy link
Author

This would not happen if I disable all plugins. It needs at least one SCM plugin to be active and QuickDiff provider to be registered.

I can reproduce this with git plugin or with my own SCM plugin I've developed. The delay and loading animation on UI is caused by repeated calls to provideOriginalResource(uri: Uri).

I've personally debugged my own plugin and came to this conclusion. Yes, I do use remote-ssh and the issue shows up prominently on remote use case. But the actual RCA seems to be that above function should not get called on every keypress. It returns a static value in most cases and calling that repeatedly serves no real purpose.

@lszomoru
Copy link
Member

@alexr00, would you be able to take a look at this since it's related to the quick diff provider?

@lszomoru lszomoru added scm General SCM compound issues and removed info-needed Issue requires more information from poster labels Jul 14, 2023
@alexr00
Copy link
Member

alexr00 commented Jul 14, 2023

Yes I will look.

@alexr00
Copy link
Member

alexr00 commented Jul 21, 2023

I went back and looked at version 1.70, and I can see that it did the same thing: call provideOriginalResource on ever editor change. This isn't a new problem.

alexr00 added a commit that referenced this issue Jul 21, 2023
@alexr00 alexr00 added this to the July 2023 milestone Jul 24, 2023
@alexr00 alexr00 added the under-discussion Issue is under discussion for relevance, priority, approach label Jul 24, 2023
@alexr00 alexr00 modified the milestones: July 2023, August 2023 Jul 24, 2023
@alexr00 alexr00 modified the milestones: August 2023, September 2023 Aug 23, 2023
alexr00 added a commit that referenced this issue Sep 8, 2023
@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Sep 8, 2023
@alexr00 alexr00 added bug Issue identified by VS Code Team member as probable bug and removed under-discussion Issue is under discussion for relevance, priority, approach labels Sep 8, 2023
@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Sep 8, 2023
@alexr00
Copy link
Member

alexr00 commented Sep 25, 2023

I've reverted the caching change as it's flawed.

@alexr00 alexr00 reopened this Sep 25, 2023
@alexr00 alexr00 modified the milestones: September 2023, Backlog Sep 25, 2023
@VSCodeTriageBot VSCodeTriageBot removed the insiders-released Patch has been released in VS Code Insiders label Sep 25, 2023
@joaomoreno
Copy link
Member

joaomoreno commented Sep 26, 2023

@alexr00 What's the issue you saw that made you revert it? I remember testing it.

@alexr00
Copy link
Member

alexr00 commented Sep 26, 2023

I was getting missing quick diffs from git. They could be triggered by:

  • Check out a branch.
  • Open a file. <At this point the quick diff base is taken, never to be changed again>For the remaining steps, do not change the file.
  • Make some changes in the file. The quick diff is correct.
  • Commit your changes. <At this point the git base that we want to use as the quick diff base has been changed, but quick diff doesn't take a new base>
  • Quick diffs for git for this file are now incorrect depending on which line you edit.

Since this is an old issue I don't want to make a new fix so close to release. I also think that the best fix would be to give extensions API to announce when their quick diff base has changed, which should go through API review. Example:

	/**
	 * A quick diff provider provides a {@link Uri uri} to the original state of a
	 * modified resource. The editor will use this information to render ad'hoc diffs
	 * within the text.
	 */
	export interface QuickDiffProvider {

		/**
		 * Provide a {@link Uri} to the original resource of any given resource uri.
		 *
		 * @param uri The uri of the resource open in a text editor.
		 * @param token A cancellation token.
		 * @returns A thenable that resolves to uri of the matching original resource.
		 */
		provideOriginalResource?(uri: Uri, token: CancellationToken): ProviderResult<Uri>;

		/**
		 * **NEW API**
		 * Announce that the original resource has changed. This will trigger the editor to ask the provider for the original resource again.
		 * This should be implmented for performance reasons - e.g. if the provider is able to detect that the original resource has changed and fire this event,
		 * then `provideOriginalResoure` doesn't need to be called frequently and instead can be called only when there's an actual change.
		 */
		onDidChange?: Event<Uri | undefined | null | void>;
	}

@lszomoru lszomoru removed their assignment Dec 12, 2023
@alexr00 alexr00 added feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug labels Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality scm General SCM compound issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants