PERF: bound Money parsed-file cache bytes and share immutable documents (#663) #807

Open
opened 2026-10-02 13:12:46 +00:00 by kayg · 4 comments
Owner

Parent audit #663. Baseline c4a61e8cf0; checked against round 7a.

M3: Money parse cache grows with file history

Status: source-confirmed; audit finding.

Evidence: crates/plugins/money/src/store.rs:239 holds a per-User
HashMap<String, ParsedFile>. parse (:432–463) adds each file and returns a
deep clone on hits. load reads all months (:501–508). Each file is limited
to 8 MiB, but the cache has no byte budget, file eviction or stale-path
cleanup. The outer 64-User LRU (:199) bounds idle handles, not their bytes;
active handles can exceed that count. Document lines retain raw strings
(calternal-money/src/codec.rs:135). The same design remains in round 7a.

Impact estimate: 100 month files at 100 KiB each need at least 9.8 MiB of
raw line text per User. Across 64 warm Users, this is at least 625 MiB,
before line structs, parsed fields and allocator overhead. A read constructs
another cloned set and ledger. At the allowed 8 MiB per file, 100 months
retain at least 800 MiB for one User. Rename/delete history can retain keys
that no longer correspond to a file.

Fix: add byte budgets to parsed file entries and the outer cache. Use shared
immutable parsed documents. Evict stale paths and least-used parsed entries
without evicting an active User's writer mutex. Keep write identity separate
from disposable parsed data. Precomputed month projections are also needed
for #687/#689, but do not replace a cache-byte limit.

Test: synthetic text-only fixtures with controlled sizes; open many months
and Users, rename/remove files, then check bytes and keys. A hot hit must
share its document. Budget results must stay exact after eviction. Never
change an existing numeric test expectation to pass this fix.

Duplicate search: all-state memory/cache/unbounded and Money. #687 is request
path parsing; #689 is counts/background work. Neither sets parse-cache bytes.

Parent audit #663. Baseline c4a61e8cf090170f35b1bed3350d9de20c83ecd5; checked against round 7a. M3: Money parse cache grows with file history Status: source-confirmed; audit finding. Evidence: crates/plugins/money/src/store.rs:239 holds a per-User HashMap<String, ParsedFile>. parse (:432–463) adds each file and returns a deep clone on hits. load reads all months (:501–508). Each file is limited to 8 MiB, but the cache has no byte budget, file eviction or stale-path cleanup. The outer 64-User LRU (:199) bounds idle handles, not their bytes; active handles can exceed that count. Document lines retain raw strings (calternal-money/src/codec.rs:135). The same design remains in round 7a. Impact estimate: 100 month files at 100 KiB each need at least 9.8 MiB of raw line text per User. Across 64 warm Users, this is at least 625 MiB, before line structs, parsed fields and allocator overhead. A read constructs another cloned set and ledger. At the allowed 8 MiB per file, 100 months retain at least 800 MiB for one User. Rename/delete history can retain keys that no longer correspond to a file. Fix: add byte budgets to parsed file entries and the outer cache. Use shared immutable parsed documents. Evict stale paths and least-used parsed entries without evicting an active User's writer mutex. Keep write identity separate from disposable parsed data. Precomputed month projections are also needed for #687/#689, but do not replace a cache-byte limit. Test: synthetic text-only fixtures with controlled sizes; open many months and Users, rename/remove files, then check bytes and keys. A hot hit must share its document. Budget results must stay exact after eviction. Never change an existing numeric test expectation to pass this fix. Duplicate search: all-state memory/cache/unbounded and Money. #687 is request path parsing; #689 is counts/background work. Neither sets parse-cache bytes.
Author
Owner

Starting work on job/webperf, based on 2f4482ded066d9c5d9c59130377907f7fd2916c9 (job/merge-round-7a). I am reading the matching audit evidence and will report the concrete finding, regression coverage, measurements, and gate output here when finished.

Starting work on `job/webperf`, based on `2f4482ded066d9c5d9c59130377907f7fd2916c9` (`job/merge-round-7a`). I am reading the matching audit evidence and will report the concrete finding, regression coverage, measurements, and gate output here when finished.
Author
Owner

Finding: the Money parse cache retained unbounded parsed files and cloned whole parsed documents on hits. Cache hits now share immutable Arc<Document> values; LRU eviction applies a 16 MiB per-User and 128 MiB process budget using a retained-AST estimate that includes line structures, plus a per-User entry cap. Added cache-sharing, pruning, byte-budget and many-short-lines tests. Rust gates are still in progress.

Finding: the Money parse cache retained unbounded parsed files and cloned whole parsed documents on hits. Cache hits now share immutable `Arc<Document>` values; LRU eviction applies a 16 MiB per-User and 128 MiB process budget using a retained-AST estimate that includes line structures, plus a per-User entry cap. Added cache-sharing, pruning, byte-budget and many-short-lines tests. Rust gates are still in progress.
Author
Owner

F1 — P1: Money document accessors retain the old return type

Owner: #807. Introduced by ccd0fa98e.
Evidence: crates/plugins/money/src/store.rs:124, :169, :177.
ParsedFile.document is now Result<Arc<Document>, MoneyError>.
Both accessors still return Result<&Document, StoreError>, but their final
as_ref().map_err(...) expression has type Result<&Arc<Document>, StoreError>.
Rust does not apply the Arc dereference through Result. This is a source-level
type error that prevents the Money crate and its server consumer from building.
No compiler output is claimed.

Fix: use as_deref() in both accessors. Check all users of ParsedFile.document
for the same transition. Keep the shared immutable document.
The same commit also leaves routes.rs:311 pushing a Document into a vector
now inferred as Vec<Arc<Document>>, and routes.rs:659 returning an Arc where
month_text requires a Document. store.rs:926 passes Vec<&Arc<Document>>
to the generic ledger function that needs elements with Borrow<Document>.
Convert borrowed inputs with as_deref(), and clone the inner Document only
where an edit needs an owned value.
Rule: CLAUDE.md requires building changes and passing crate gates. DESIGN §48
requires exact Money results; no numeric expectation must change for this fix.
Test idea: run the Money crate Clippy and test gates, including existing edit,
eviction and exact-total tests. Search before reporting: Arc Document and #807.
Use #807 for this shared fix; do not create a second owner.

## F1 — P1: Money document accessors retain the old return type Owner: #807. Introduced by `ccd0fa98e`. Evidence: `crates/plugins/money/src/store.rs:124`, `:169`, `:177`. `ParsedFile.document` is now `Result<Arc<Document>, MoneyError>`. Both accessors still return `Result<&Document, StoreError>`, but their final `as_ref().map_err(...)` expression has type `Result<&Arc<Document>, StoreError>`. Rust does not apply the Arc dereference through Result. This is a source-level type error that prevents the Money crate and its server consumer from building. No compiler output is claimed. Fix: use `as_deref()` in both accessors. Check all users of `ParsedFile.document` for the same transition. Keep the shared immutable document. The same commit also leaves `routes.rs:311` pushing a `Document` into a vector now inferred as `Vec<Arc<Document>>`, and `routes.rs:659` returning an Arc where `month_text` requires a Document. `store.rs:926` passes `Vec<&Arc<Document>>` to the generic ledger function that needs elements with `Borrow<Document>`. Convert borrowed inputs with `as_deref()`, and clone the inner Document only where an edit needs an owned value. Rule: CLAUDE.md requires building changes and passing crate gates. DESIGN §48 requires exact Money results; no numeric expectation must change for this fix. Test idea: run the Money crate Clippy and test gates, including existing edit, eviction and exact-total tests. Search before reporting: `Arc Document` and #807. Use #807 for this shared fix; do not create a second owner.
Author
Owner

Finding and fix

The Arc<Document> migration left borrowed accessors typed as &Arc<Document> and mixed Arc<Document> with owned Document values in route projections. I changed the borrowed paths to as_deref(), cloned the inner document only when a request needs an owned editable value, and converted ledger inputs to &Document.

The generated Money property test now compares the exact serialized ledger from owned and shared Arc<Document> month inputs across random transaction sets. The number rules and expectations did not change.

Verification

cargo clippy -p calternal-plugin-money --all-targets -- -D warnings:

Finished `dev` profile [unoptimized + debuginfo] target(s) in 6m 48s

cargo test -p calternal-plugin-money:

test result: ok. 28 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 73.98s

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
## Finding and fix The `Arc<Document>` migration left borrowed accessors typed as `&Arc<Document>` and mixed `Arc<Document>` with owned `Document` values in route projections. I changed the borrowed paths to `as_deref()`, cloned the inner document only when a request needs an owned editable value, and converted ledger inputs to `&Document`. The generated Money property test now compares the exact serialized ledger from owned and shared `Arc<Document>` month inputs across random transaction sets. The number rules and expectations did not change. ## Verification `cargo clippy -p calternal-plugin-money --all-targets -- -D warnings`: ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 6m 48s ``` `cargo test -p calternal-plugin-money`: ```text test result: ok. 28 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 73.98s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ```
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#807
No description provided.