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

perf: overall prevention of duplicate rendering in localSearch #3170

Merged
merged 5 commits into from Nov 5, 2023

Conversation

zonemeen
Copy link
Collaborator

@zonemeen zonemeen commented Nov 2, 2023

The cache should be defined outside of debouncedWatch, otherwise a new cache will be redefined every time when debouncedWatch is retriggered. It's a bit of a waste of performance.

@brc-dd
Copy link
Member

brc-dd commented Nov 3, 2023

This will work, but will also cause memory leaks and that cache will keep on growing. We probably should use LRU cache here with some max limit like 256 pages. Instead of using lru-cache (which adds up extra 15kB), IG we simply can do something like https://stackoverflow.com/a/46432113/11613622

@zonemeen
Copy link
Collaborator Author

zonemeen commented Nov 3, 2023

Thanks for the advice. I'll change it later when I have time.

@brc-dd brc-dd merged commit 878f437 into vuejs:main Nov 5, 2023
7 checks passed
@zonemeen
Copy link
Collaborator Author

zonemeen commented Nov 6, 2023

@brc-dd Nice work on the next step! There's also a bit of a question here about whether 16 cache entries would be a bit too few.

@zonemeen zonemeen deleted the perf/localSearch branch November 6, 2023 01:28
@brc-dd
Copy link
Member

brc-dd commented Nov 6, 2023

Ah, 16 here actually means 16 files in memory. So there will be like 16 maps having anchor to rendered html matching which will already be quite big. And I tested this on our docs, the cache misses were very few. On larger sites, that ratio might be slightly higher, but won't be there if the user is searching related stuff.

@zonemeen
Copy link
Collaborator Author

zonemeen commented Nov 7, 2023

Ah, got it! 🤗

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants