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

Plug-ins leaking CodeLens, etc. #4472 #7238

Merged
merged 1 commit into from
Mar 2, 2020

Conversation

vrubezhny
Copy link
Contributor

@vrubezhny vrubezhny commented Feb 27, 2020

Fixes: #4472

Signed-off-by: Victor Rubezhny vrubezhny@redhat.com

What it does

For the following adapters :
CodeLensAdapter
LinkProviderAdapter

the releaseXXX-like methods are added in order to release the cached items when dispose() is invoked on the objects provided for the according provider, These dispose() methods now are set via provideCodeLenses and provideLinks methods defined in LanguagesMainImpl.

TaskProviderAdapter is cleared of any result caching because, according to https://code.visualstudio.com/api/references/vscode-api#TaskProvider, the resolveTask method will not be called for tasks returned from the above provideTasks method since those tasks are always fully resolved

All the methods/calls are already made for the CompletionAdapter, so no modification is needed here

Note that the proper releasing of the cache maintained in LinkProviderAdapter the fix microsoft/vscode@e59cb58 for the following issue microsoft/vscode#91536 is to be picked up into the build.

How to test

#7238 (comment)

Review checklist

Reminder for reviewers

@akosyakov
Copy link
Member

Please feel in how to test section. One has to inspect the plugin host twice to confirm that lenses are cached and second time that they been removed after editing. For task we just should check that there are no regressions. For both cases short instruction what a user can do is helpful now and in the future. In docs folder there should be an explanation how to inspect the plugin host process.

@akosyakov akosyakov added performance issues related to performance plug-in system issues related to the plug-in system labels Feb 28, 2020
@vrubezhny
Copy link
Contributor Author

vrubezhny commented Feb 28, 2020

Two tests were made for CodeLens-es cached in CodeLensAdapter - one using master branch and another one using this PR.
A simple java console application project was used in order to see Run | Debug Code Lenses generated for public static void main(String... argvs) methos in Java Editor, then editing its name (so, the Lenses should have disappeared ) and then changing its name back to main. Such a cycle was repeated about ten times. Then the editor was closed.
For the master branch it clearly show a lot of CodeLens objects contained in the CodeLensAdapter cache:
master
Snapshot made while testing the master branch: https://github.com/vrubezhny/test-storage/blob/master/et4472/master-Heap-20200228T140014.heaptimeline
While for this PR the cache gets cleared after during the testing as well as after the editor is closed:
et4472
Snapshot made while testing the PR #7238: https://github.com/vrubezhny/test-storage/blob/master/et4472/et4472-Heap-20200228T135139.heaptimeline

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

profiled reference code lens editing with java extension and it seems to work fine, there are some weirdness that they are not released immediately sometimes, but it is because of Monaco timings, eventually they get released

Fixes: eclipse-theia#4472

Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
@akosyakov
Copy link
Member

@vrubezhny do you have write access?

@vrubezhny
Copy link
Contributor Author

@vrubezhny do you have write access?

No, I don't

@akosyakov
Copy link
Member

We will merge your PRs this time and nominate you as a committer afterwards based on them.

@akosyakov akosyakov merged commit 6d0abe5 into eclipse-theia:master Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance issues related to performance plug-in system issues related to the plug-in system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plug-ins leaking CodeLens, etc.
2 participants