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

Ensure all dependencies in a multi-root project are cached #641

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dmakovec
Copy link

I've had issues with a multi-root project where CPU runs through the roof every time https://github.com/felixfbecker/vscode-php-intellisense is opened.

The reason for this was LanguageServer.php was arbitrarily choosing the first composer.lock that it came across within rootPath, then indexing and caching dependencies from it alone. Consequently, whenever VSCode restarts, php-language-server re-indexes all of the vendor packages in sub-folders that weren't associated with the selected composer.json/lock file.

My quick patch here simply iterates over all composer.lock files within the multi-root project, indexes then caches to disk every single dependency it uncovers. The result of this being that subsequent startup times are much faster and CPU no longer tanks.

This is my simple solution which I believe partially solves what #509 is attempting to address. Possibly related to #465

Dan Makovec and others added 2 commits May 10, 2018 13:04
@codecov
Copy link

codecov bot commented Sep 8, 2018

Codecov Report

Merging #641 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##             master    #641      +/-   ##
===========================================
- Coverage     81.52%   81.5%   -0.02%     
+ Complexity      912     884      -28     
===========================================
  Files            62      44      -18     
  Lines          2078    2001      -77     
===========================================
- Hits           1694    1631      -63     
+ Misses          384     370      -14
Impacted Files Coverage Δ Complexity Δ
src/LanguageServer.php 72.88% <0%> (-5.31%) 28 <0> (+1)
src/DefinitionResolver.php 87.32% <0%> (ø) 330% <0%> (ø) ⬇️
src/TreeAnalyzer.php 94.28% <0%> (ø) 53% <0%> (ø) ⬇️
src/Server/TextDocument.php 75.37% <0%> (ø) 56% <0%> (ø) ⬇️
src/Server/Workspace.php 38.46% <0%> (ø) 23% <0%> (ø) ⬇️
src/CompletionProvider.php 94.51% <0%> (ø) 108% <0%> (ø) ⬇️
src/Protocol/ParameterInformation.php
src/Protocol/Message.php
src/Protocol/TextEdit.php
src/Protocol/SignatureHelp.php
... and 24 more

}
$this->composerLock = $composerLock;
$this->composerLock->packages = $packages;
$this->composerLock->{'packages-dev'} = $packagesDev;
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, are you sure aggregating all packages work? What if the same package appears with different versions, and then the cache key that is used for the index is not actually the package version that was indexed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants