X Tutup
Skip to content

Per document cache#2882

Open
dantleech wants to merge 2 commits intomasterfrom
per-document-cache
Open

Per document cache#2882
dantleech wants to merge 2 commits intomasterfrom
per-document-cache

Conversation

@dantleech
Copy link
Collaborator

@dantleech dantleech commented May 3, 2025

  • Use an actively invalidated per-document cache for AST parsing and type resolving.
  • Allow the highlight functionality to be toggled

The node resolver cache isn't so effective when ediring a document as the AST is reparsed on each edit and the cache invalidated - so at best it's the same as it was before in the current document however some operations analyse other documents - in which case it's very beneficial.

Caching the AST also ensures that the object_ids stay the same and the cache remains valid.#

The cache is purged for each document when it is either updated, closed or saved.

Danger:

  • increases memory usage significantly - but it should be bound to the number of open documents rather than growing over time.

Problems

The following should be Action&DeleteAction but it alternates between that and the parameter type:
image
image
When it is correct goto-definition jumps to the parameter type when it should offer a choice between the intersection types.

@dantleech dantleech force-pushed the per-document-cache branch from f90ee39 to 40946b7 Compare May 8, 2025 22:01
- Per-document caching for the AST
- Per-document caching for the node resolver
- Stats for cache hits etc.
- Adds option to disable highlighter
- Log cache purges
@dantleech dantleech force-pushed the per-document-cache branch from 40946b7 to db3477b Compare June 28, 2025 14:23
public function findIn(TextDocument $source): NamespacedClassReferences
{
$ast = $this->parser->parseSourceFile($source->__toString());
$ast = $this->parser->parseSourceFile($source->__toString(), $source->uri()?->__toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing that we use it like this most of the time, why not change it to take in the whole source.

@przepompownia
Copy link
Contributor

I have locally merged this branch into my experimental branch rather to catch if something can fail than to observe the impact on performance.

@dantleech do you consider to add any benchmark here?


namespace Phpactor;

final class Stats
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

opentelemetry will do this better, remove.

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.

3 participants

X Tutup