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

Add ability to gc node caches #63

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

Conversation

CAD97
Copy link
Collaborator

@CAD97 CAD97 commented Jun 19, 2020

Originally implemented in sorbus here: CAD97/sorbus#46

Also exposes the NodeCache (closes #53) so that the gc is usable. While GreenNodeBuilder has had with_cache for a while, it's been impossible to actually share the cache, as there was no way to get a cache to share!

@CAD97 CAD97 requested a review from matklad June 19, 2020 02:14
nodes: FxHashSet<GreenNode>,
tokens: FxHashSet<GreenToken>,
nodes: HashMap<GreenNode, ()>,
tokens: HashMap<GreenToken, ()>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these HashMap just because of drain_filter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes (unfortunately).

A further optimization step in the pipeline will take further advantage of HashMap::raw_entry_mut, so the switch to using set-like maps is probably inevitable, though.

@simonvandel
Copy link

What is the effect on peak memory usage for analysis stats using rust analyzer?

@CAD97
Copy link
Collaborator Author

CAD97 commented Jun 20, 2020

It's not going to have an immediate impact because r-a doesn't (IIUC) yet hold on to or reuse node caches.

Additionally, it will only potentially have an impact when holding onto a node cache after the deletion of a parse tree. We aren't really generating any garbage to clean up; it's only useful if the cache outlives the tree(s) that it's built.

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

Successfully merging this pull request may close these issues.

Exporting some version of Cache
3 participants