Conversation
Member
|
Thanks for the PR. I think the general idea of being more careful with freeing up memory is sensible. That said, we generally don't merge AI-generated PRs and this PR shows various signs of being generated. We don't currently have dedicated details about our AI policy in typst/comemo (only in typst/typst), but we'll look into adjusting that. |
Author
|
No worries, i was using tinymist and over a day of usage the ram allocation grew to 30GiB (kinda making it unusable on most machines) and i had no time to look into it much, hence i let AI do the triage. It found something and i thought either, i leave it be or let you know what it found. Sorry for the inconvenience. I don't want to cause any extra work, but i hoped somebody would look into it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eviction removes cached entries but retains their backing allocations. In a long-running Tinymist process, empty accelerator maps and their pool retained approximately 11.62 GiB after cache clearing.
This change releases accelerator storage on
evict(0), releases unused accelerator map buffers during ordinary eviction, and resets empty call trees. Accelerator lookup rechecks ID validity and pool length across lock transitions because full eviction can now shrink the pool.Recently used buffers remain reusable, and eviction frequency is unchanged. Nonempty trees are not compacted. This keeps the change to 38 added and 20 removed production lines, plus three regression tests for storage release, reinsertion, and concurrent access/eviction.
Validation:
QUICKCHECK_TESTS=10000 cargo test --all-features -- --test-threads=1.The replay is a single workload measurement using a private document; the regression tests exercise storage retention independently. glibc still retained free arena pages, so releasing these containers does not eliminate all RSS retention. Tests used Rust 1.98.1; the expanded suite ran serially because existing global-eviction tests can interfere with concurrent property tests.
Related history: Tinymist #161 increased eviction frequency and was reverted in #173 because of cache thrashing. This change keeps the eviction schedule intact.