CP4.2: review — pack dictionaries share a memory budget and a paragraph never evicts its own, so many languages load once

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FypgCX8LsYYE8fsj4byBm2

crates/proof/README.md

@@ -92,8 +92,7 @@ through the shared affix engine with default tuning, and hyphenated from its pac
 `hyphenation.pat`; it has no grammar rules and the generic English spelling message. Without a
 dictionary or pack it behaves exactly as `Other` did (no marks, English hyphenation fallback).
 `has_dictionary(tag)` / `ProofLang::is_proofed()` say whether a `Pack` language has a dictionary
-now (read on first use, cached). At most 8 `Pack` dictionaries stay loaded (least recently used
-goes first). Downstream `match`es on `ProofLang` need a `Pack(_)` arm; `code()` is `""` for it, as
+now (read on first use, cached). Loaded `Pack` dictionaries share a 256 MB budget (estimated from their files); the least recently used goes first, but never one used in the paragraph being checked, and languages with no dictionary take no room.  Downstream `match`es on `ProofLang` need a `Pack(_)` arm; `code()` is `""` for it, as
 for `Other` (use `pack_locale()`).
 
 **Which pack serves a language:** its locale, then shorter prefixes (`fr-ca` → `fr`), then another

crates/proof/src/lib.rs

@@ -896,6 +896,12 @@ pub fn check_text(text: &str, spans: &[(Range<usize>, ProofLang)]) -> Vec<Issue>
 /// `list_item` marks a list paragraph: items are often fragments, so the English rule asking for
 /// a capital on the paragraph's first word is off for it (later sentences are still checked).
 pub fn check_text_with(text: &str, spans: &[(Range<usize>, ProofLang)], opts: &Options, grammar: bool, list_item: bool) -> Vec<Issue> {
+    // A paragraph in many pack languages loads each dictionary once.
+    pack_lang::in_one_check(|| check_text_once(text, spans, opts, grammar, list_item))
+}
+
+/// [`check_text_with`], inside one [`pack_lang::in_one_check`].
+fn check_text_once(text: &str, spans: &[(Range<usize>, ProofLang)], opts: &Options, grammar: bool, list_item: bool) -> Vec<Issue> {
     // Sorted, empty ones dropped, neighbours of one language merged.
     let mut sorted: Vec<&(Range<usize>, ProofLang)> = spans.iter().filter(|(r, _)| r.start < r.end).collect();
     sorted.sort_by_key(|s| s.0.start);
@@ -1527,26 +1533,71 @@ mod tests {
         let _ = std::fs::remove_dir_all(&root);
     }
 
-    /// At most [`pack_lang::MAX_LOADED`] pack-language dictionaries stay loaded; others load again
-    /// when used.
+    /// Pack languages for the cache tests: `qa`, `qb`… each knowing `word<locale>`.
+    fn many_packs(root: &std::path::Path, n: usize) -> Vec<ProofLang> {
+        (0..n)
+            .map(|i| {
+                let l = format!("q{}", (b'a' + i as u8) as char);
+                write_pack(root, &l, &[("spelling.aff", b"SET UTF-8\n"), ("spelling.dic", format!("1\nword{l}\n").as_bytes())]);
+                ProofLang::from_tag(Some(&l))
+            })
+            .collect()
+    }
+
+    /// Review of #485: ten pack languages used in turn stay loaded (a memory budget, not eight
+    /// entries), so switching between them never reads a dictionary again.
     #[test]
-    fn pack_language_dictionaries_are_capped() {
+    fn many_pack_languages_load_once() {
         let _guard = DICTIONARY_TESTS.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
-        // The test dictionaries on disk first, so a reload never leaves other tests without them.
         set_dictionaries_folder(Some(shared_test_folder()));
         let root = std::env::temp_dir().join(format!("wc-proof-many-packs-{}", std::process::id()));
         let _ = std::fs::remove_dir_all(&root);
-        let locales: Vec<String> = (0..pack_lang::MAX_LOADED + 3).map(|i| format!("q{}", (b'a' + i as u8) as char)).collect();
-        for l in &locales {
-            write_pack(&root, l, &[("spelling.aff", b"SET UTF-8\n"), ("spelling.dic", format!("1\nword{l}\n").as_bytes())]);
-        }
+        let langs = many_packs(&root, 10);
         packs::set_packs_dir(Some(root.clone()));
-        for _ in 0..2 {
-            for l in &locales {
-                let lang = ProofLang::from_tag(Some(l));
-                assert!(is_correct_in(&format!("word{l}"), lang) && !is_correct_in("wordzz", lang), "{l}");
+        let loads = pack_lang::loads();
+        let start = std::time::Instant::now();
+        // A paragraph of 20 words over the ten languages, checked 50 times.
+        let mut text = String::new();
+        let mut spans = Vec::new();
+        for k in 0..20 {
+            let lang = langs[k % 10];
+            let w = format!("word{} ", lang.pack_locale().unwrap());
+            spans.push((text.len()..text.len() + w.len(), lang));
+            text.push_str(&w);
+        }
+        for _ in 0..50 {
+            assert!(check_text(&text, &spans).is_empty());
+            for lang in &langs {
+                assert!(!is_correct_in("wordzz", *lang));
             }
         }
+        assert_eq!(pack_lang::loads() - loads, 10, "each dictionary is read once");
+        assert!(start.elapsed() < std::time::Duration::from_secs(10), "{:?}", start.elapsed());
+        // A language with no dictionary takes no room and is not looked up again.
+        assert!(is_correct_in("anything", ProofLang::from_tag(Some("qz"))));
+        packs::set_packs_dir(None);
+        set_dictionaries_folder(Some(shared_test_folder()));
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// Review of #485: over the memory budget, a dictionary used during the current paragraph is
+    /// never evicted, so the paragraph loads each once.
+    #[test]
+    fn a_paragraph_never_evicts_its_own_dictionaries() {
+        let _guard = DICTIONARY_TESTS.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
+        set_dictionaries_folder(Some(shared_test_folder()));
+        let root = std::env::temp_dir().join(format!("wc-proof-budget-packs-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&root);
+        let langs = many_packs(&root, 2);
+        packs::set_packs_dir(Some(root.clone()));
+        pack_lang::set_budget(1);
+        let (a, b) = (langs[0], langs[1]);
+        let text = "wordqa wordqb wordqa wordqb wordqa wordqb";
+        let spans: Vec<(Range<usize>, ProofLang)> = (0..6).map(|k| (k * 7..k * 7 + 6, if k % 2 == 0 { a } else { b })).collect();
+        let loads = pack_lang::loads();
+        assert!(check_text(text, &spans).is_empty());
+        assert_eq!(pack_lang::loads() - loads, 2);
+        pack_lang::set_budget(pack_lang::MAX_LOADED_BYTES);
         packs::set_packs_dir(None);
         set_dictionaries_folder(Some(shared_test_folder()));
         let _ = std::fs::remove_dir_all(&root);

crates/proof/src/pack_lang.rs

@@ -2,42 +2,91 @@
 //! dictionaries folder, then an installed Language pack, then the system's), read by the shared
 //! affix engine, with default suggestion tuning and no grammar rules.
 //!
-//! At most [`MAX_LOADED`] dictionaries stay loaded; the one used least recently goes first.
+//! The loaded dictionaries share a memory budget ([`MAX_LOADED_BYTES`], estimated from their
+//! files); the one used least recently goes first, but never one used during the current
+//! [`in_one_check`] call, so a paragraph in many languages loads each dictionary once. Locales
+//! with no dictionary are remembered apart and take no room.
 
+use std::cell::Cell;
 use std::path::PathBuf;
+use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::{Arc, PoisonError, RwLock};
 
 use crate::affix::AffixDict;
 use crate::affix_suggest::Tuning;
 use crate::lang::PackLocale;
 
-/// Most locale dictionaries kept loaded at once.
-pub(crate) const MAX_LOADED: usize = 8;
+/// The memory budget of the loaded dictionaries, in estimated bytes.
+pub(crate) const MAX_LOADED_BYTES: u64 = 256 << 20;
+/// Estimated memory per byte of `.aff` and `.dic` file.
+const BYTES_PER_FILE_BYTE: u64 = 6;
+/// Most locales remembered as having no dictionary (the list is cleared when full).
+const MAX_MISSING: usize = 256;
+
+/// The budget in force (tests lower it).
+static BUDGET: AtomicU64 = AtomicU64::new(MAX_LOADED_BYTES);
+/// Dictionaries loaded from disk so far (tests count them).
+static LOADS: AtomicU64 = AtomicU64::new(0);
+
+thread_local! {
+    /// The clock when this thread's current [`in_one_check`] call started.
+    static CHECK_FROM: Cell<Option<u64>> = const { Cell::new(None) };
+}
 
 struct Loaded {
     locale: PackLocale,
     dict: Arc<AffixDict>,
-    /// The `.dic` it came from (`None`: none found, the language is unchecked).
+    /// The `.dic` it came from.
     file: Option<PathBuf>,
+    /// Estimated memory.
+    bytes: u64,
     used: u64,
 }
 
 struct Cache {
     loaded: Vec<Loaded>,
+    /// Locales with no dictionary (or one that failed to load).
+    missing: Vec<PackLocale>,
     clock: u64,
     /// Counts reloads, so a load that started before one is not kept.
     reloads: u64,
 }
 
-static CACHE: RwLock<Cache> = RwLock::new(Cache { loaded: Vec::new(), clock: 0, reloads: 0 });
+static CACHE: RwLock<Cache> = RwLock::new(Cache { loaded: Vec::new(), missing: Vec::new(), clock: 0, reloads: 0 });
 
 /// Forget every loaded dictionary (the dictionaries folder or the packs changed).
 pub(crate) fn reload() {
     let mut c = CACHE.write().unwrap_or_else(PoisonError::into_inner);
     c.loaded.clear();
+    c.missing.clear();
     c.reloads = c.reloads.wrapping_add(1);
 }
 
+/// Run `f` as one check (a paragraph): no dictionary it uses is evicted before it returns, even
+/// over the budget.
+pub(crate) fn in_one_check<R>(f: impl FnOnce() -> R) -> R {
+    let outer = CHECK_FROM.with(Cell::get);
+    if outer.is_none() {
+        let now = CACHE.read().unwrap_or_else(PoisonError::into_inner).clock;
+        CHECK_FROM.with(|c| c.set(Some(now.wrapping_add(1))));
+    }
+    let r = f();
+    if outer.is_none() {
+        CHECK_FROM.with(|c| c.set(None));
+    }
+    r
+}
+
+#[cfg(test)]
+pub(crate) fn loads() -> u64 {
+    LOADS.load(Ordering::Relaxed)
+}
+
+#[cfg(test)]
+pub(crate) fn set_budget(bytes: u64) {
+    BUDGET.store(bytes, Ordering::Relaxed);
+}
+
 /// Hunspell file names for `locale`, best first: `fr-fr` → `fr_FR`, `fr-FR`, `fr`;
 /// `sr-latn` → `sr_Latn`, `sr-Latn` (never plain `sr`, another script).
 pub(crate) fn names(locale: PackLocale) -> Vec<String> {
@@ -89,35 +138,56 @@ fn get(locale: PackLocale) -> (Arc<AffixDict>, Option<PathBuf>) {
             l.used = now;
             return (l.dict.clone(), l.file.clone());
         }
+        if c.missing.contains(&locale) {
+            return (Arc::default(), None);
+        }
         c.reloads
     };
-    let (dict, file) = load(locale);
+    let (dict, file, bytes) = load(locale);
     let mut c = CACHE.write().unwrap_or_else(PoisonError::into_inner);
-    if c.reloads == reloads && !c.loaded.iter().any(|l| l.locale == locale) {
-        if c.loaded.len() >= MAX_LOADED
-            && let Some(oldest) = c.loaded.iter().enumerate().min_by_key(|(_, l)| l.used).map(|(i, _)| i)
-        {
-            c.loaded.swap_remove(oldest);
-        }
-        let used = c.clock;
-        c.loaded.push(Loaded { locale, dict: dict.clone(), file: file.clone(), used });
+    if c.reloads != reloads || c.loaded.iter().any(|l| l.locale == locale) || c.missing.contains(&locale) {
+        return (dict, file);
     }
+    if dict.is_empty() {
+        if c.missing.len() >= MAX_MISSING {
+            c.missing.clear();
+        }
+        c.missing.push(locale);
+        return (dict, file);
+    }
+    LOADS.fetch_add(1, Ordering::Relaxed);
+    let budget = BUDGET.load(Ordering::Relaxed);
+    let pinned_from = CHECK_FROM.with(Cell::get).unwrap_or(u64::MAX);
+    let mut total: u64 = c.loaded.iter().map(|l| l.bytes).sum();
+    while total.saturating_add(bytes) > budget {
+        // The least recently used dictionary not used during this check.
+        let Some(oldest) = c.loaded.iter().enumerate().filter(|(_, l)| l.used < pinned_from).min_by_key(|(_, l)| l.used).map(|(i, _)| i) else {
+            break;
+        };
+        let gone = c.loaded.swap_remove(oldest);
+        total = total.saturating_sub(gone.bytes);
+    }
+    let used = c.clock;
+    c.loaded.push(Loaded { locale, dict: dict.clone(), file: file.clone(), bytes, used });
     (dict, file)
 }
 
-fn load(locale: PackLocale) -> (Arc<AffixDict>, Option<PathBuf>) {
+/// Load `locale`'s dictionary, with its estimated memory.
+fn load(locale: PackLocale) -> (Arc<AffixDict>, Option<PathBuf>, u64) {
     let names = names(locale);
     let refs: Vec<&str> = names.iter().map(String::as_str).collect();
     let key = names.first().map_or("", String::as_str);
     let Some((aff, dic)) = crate::hunspell::find(&refs) else {
         crate::hunspell::clear_load_error(key);
-        return (Arc::default(), None);
+        return (Arc::default(), None, 0);
     };
+    let size = |p: &std::path::Path| std::fs::metadata(p).map_or(0, |m| m.len());
+    let bytes = size(&aff).saturating_add(size(&dic)).saturating_mul(BYTES_PER_FILE_BYTE).max(1);
     match crate::hunspell::note_load(key, &dic, crate::hunspell::load(&aff, &dic)) {
-        Ok(d) => (Arc::new(d), Some(dic)),
+        Ok(d) => (Arc::new(d), Some(dic), bytes),
         Err(e) => {
             log::warn!("{locale} spelling: {e}");
-            (Arc::default(), None)
+            (Arc::default(), None, 0)
         }
     }
 }