Skip to content

Only hold content for open files in memory #60

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

Closed
wants to merge 3 commits into from

Conversation

felixfbecker
Copy link
Owner

Addressing #57 (comment)

Content is only set on didOpen, and removed on didClose.
For indexing only the AST is kept.

It didn't reduce RAM usage unfortunately.

@codecov-io
Copy link

codecov-io commented Oct 10, 2016

Current coverage is 90.67% (diff: 94.73%)

Merging #60 into master will increase coverage by 0.75%

@@             master        #60   diff @@
==========================================
  Files            23         23          
  Lines           496        504     +8   
  Methods          70         73     +3   
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            446        457    +11   
+ Misses           50         47     -3   
  Partials          0          0          
Diff Coverage File Path
••••••••• 93% src/PhpDocument.php
•••••••••• 100% src/LanguageServer.php
•••••••••• 100% src/Server/TextDocument.php

Powered by Codecov. Last update e75c159...3cf1fd9

@mniewrzal
Copy link
Contributor

mniewrzal commented Oct 10, 2016

I think you need unset $statements too with removeContent() and not keep them in PhpDocument after parse(). Memory saving for magento after removing content from PhpDocument on close what just about 20% (630MiB => 500 MiB).

@felixfbecker
Copy link
Owner Author

Yeah but that will not work with current implementation of definition index, they reference AST node, which in turn references parent node, etc.

20% is at least something.

I will look into completely removing the AST and only keeping an index of FQN => PhpDocument, then reparse the document on demand.

@felixfbecker
Copy link
Owner Author

Can we merge #35? Need to use uriToPath function

@mniewrzal
Copy link
Contributor

Can we merge #35? Need to use uriToPath function

Yes, will you do rebase?

@felixfbecker
Copy link
Owner Author

Actually let's merge this first, doesn't hurt. Any objections?

@mniewrzal
Copy link
Contributor

Nothing from my side.

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

Successfully merging this pull request may close these issues.

3 participants