Skip to content

Work around VS Code specific performance issues#11657

Merged
feerrenrut merged 3 commits into
nvaccess:betafrom
LeonarddeR:codeNew
Sep 23, 2020
Merged

Work around VS Code specific performance issues#11657
feerrenrut merged 3 commits into
nvaccess:betafrom
LeonarddeR:codeNew

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Sep 22, 2020

Copy link
Copy Markdown
Collaborator

Note, I filed this against beta as I feel that this issue has impact for many developers and therefore deserves urgency. Furthermore, the impact outside code is zero.

Link to issue number:

Fixes #11533
Replaces #11653

Summary of the issue:

Visual Studio Code can be very sluggish. The reason is that the root of the Electron application has a tree interceptor that only contains an application, and therefore is basically doing nothing. However, when NVDA receives an event inside Visual Studio Code, it tries to find the tree interceptor assigned to the event's object. It also queries the unused tree interceptor, which is very costly.

Description of how this pull request fixes the issue:

Reintroduced appmodules as suggested by #11653 (comment) .

Testing performed:

Tested that performance increased significantly in both code and code insiders.

Known issues with pull request:

  1. When you enable browse mode in the VS Code application using NVDA+space and then disable it, performance degrades a little. This is solved by alt+tab out and back in.
  2. Not general fix, as such Performance issues caused by virtual buffers when many events are fired #11652 remains. This is a VS Code specific workaround. Ideally, the mentioned issue should be investigated further, but this issue has too much impact for end users to be left alone for a better fix.

Change log entry:

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 7d0491fb15

@feerrenrut

Copy link
Copy Markdown
Contributor

Despite it being a (very) late inclusion, we'll include this in 2020.3.

  • This will only affect VS Code users
  • Developers are more likely to notify us of issues.
  • This will be easy to revert if a problem comes up
  • No translations are affected, meaning it could be reverted even after translation freeze.

michaelDCurran
michaelDCurran previously approved these changes Sep 23, 2020

@michaelDCurran michaelDCurran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I support this being included in 2020.3.

@michaelDCurran

michaelDCurran commented Sep 23, 2020 via email

Copy link
Copy Markdown
Member

feerrenrut
feerrenrut previously approved these changes Sep 23, 2020

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LeonarddeR

@feerrenrut feerrenrut merged commit 18c737b into nvaccess:beta Sep 23, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Sep 23, 2020
@akash07k

Copy link
Copy Markdown
Contributor

Thanks @LeonarddeR
I'll test and will provide the feedback

@akash07k

akash07k commented Oct 2, 2020

Copy link
Copy Markdown
Contributor

@LeonarddeR
Thanks a lot buddy.
Testing it since many days and the performance is really improved.
Thanks a tons for your efforts in this.

@LeonarddeR LeonarddeR deleted the codeNew branch August 23, 2025 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants