From 19573438cf43b2e0286be8c2fb30cfc9cc74b08f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 19:37:52 +0000 Subject: [PATCH] Bound the regex and glob caches [patch] Both caches are keyed by pattern text and had no eviction, expiry or size cap, so every distinct pattern ever passed in stayed for the lifetime of the process. In the keystroke-driven filter box this library exists for, every prefix a user types is a distinct key, and the case-sensitivity prefix on the key doubled the space again - so the caches grew with everything ever typed rather than with the data being filtered. Route both inserts through AddBounded, which clears at a 4096-entry cap. Clear-and-restart rather than LRU because ConcurrentDictionary keeps no eviction order and tracking one would put a write on every cache hit, the path the cache exists to keep cheap. The check runs only on a miss, where a compile is about to happen anyway. Fixes #103 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Pup1E96GmvMiRTNMdABat --- TextFilter.Test/TextFilterTests.cs | 49 ++++++++++++++++++++++++++++++ TextFilter/TextFilter.cs | 29 ++++++++++++++++-- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/TextFilter.Test/TextFilterTests.cs b/TextFilter.Test/TextFilterTests.cs index 310d096..562fc2d 100644 --- a/TextFilter.Test/TextFilterTests.cs +++ b/TextFilter.Test/TextFilterTests.cs @@ -666,4 +666,53 @@ public void AnOrdinaryRegexIsUnaffectedByTheTimeout() Assert.IsTrue(TextFilter.IsMatch("hello world", "^hello", TextFilterType.Regex, TextFilterMatchOptions.ByWholeString)); Assert.IsFalse(TextFilter.IsMatch("hello world", "^goodbye", TextFilterType.Regex, TextFilterMatchOptions.ByWholeString)); } + + [TestMethod] + public void TheRegexCacheStaysBoundedAsDistinctPatternsArrive() + { + // The keystroke case from the report: every prefix a user types is a distinct key. Feeding + // more distinct patterns than the cap must not leave more than the cap cached. + for (int i = 0; i < (TextFilter.MaxCacheEntries * 2); i++) + { + TextFilter.IsMatch("hello world", $"^p{i}x", TextFilterType.Regex, TextFilterMatchOptions.ByWholeString); + } + + Assert.IsLessThanOrEqualTo( + TextFilter.MaxCacheEntries, + TextFilter.RegexCacheCount, + "The regex cache should be bounded, not grow with every pattern ever seen."); + } + + [TestMethod] + public void TheGlobCacheStaysBoundedAsDistinctPatternsArrive() + { + for (int i = 0; i < (TextFilter.MaxCacheEntries * 2); i++) + { + TextFilter.IsMatch("hello world", $"*q{i}z*", TextFilterType.Glob, TextFilterMatchOptions.ByWholeString); + } + + Assert.IsLessThanOrEqualTo( + TextFilter.MaxCacheEntries, + TextFilter.GlobCacheCount, + "The glob cache should be bounded, not grow with every pattern ever seen."); + } + + [TestMethod] + public void ACachedPatternStillMatchesAfterTheCacheHasReset() + { + // Guards the over-correction: a bound that dropped correctness rather than entries. The same + // pattern must answer identically before and after enough churn to force a reset. + const string pattern = "^hello"; + Assert.IsTrue(TextFilter.IsMatch("hello world", pattern, TextFilterType.Regex, TextFilterMatchOptions.ByWholeString)); + + for (int i = 0; i < (TextFilter.MaxCacheEntries + 1); i++) + { + TextFilter.IsMatch("hello world", $"^r{i}y", TextFilterType.Regex, TextFilterMatchOptions.ByWholeString); + } + + Assert.IsTrue( + TextFilter.IsMatch("hello world", pattern, TextFilterType.Regex, TextFilterMatchOptions.ByWholeString), + "A pattern evicted by the bound should simply be recompiled, not answer differently."); + Assert.IsFalse(TextFilter.IsMatch("hello world", "^goodbye", TextFilterType.Regex, TextFilterMatchOptions.ByWholeString)); + } } diff --git a/TextFilter/TextFilter.cs b/TextFilter/TextFilter.cs index 1cf1c68..705b9d3 100644 --- a/TextFilter/TextFilter.cs +++ b/TextFilter/TextFilter.cs @@ -84,6 +84,31 @@ public static partial class TextFilter private static ConcurrentDictionary RegexCache { get; } = []; private static ConcurrentDictionary GlobCache { get; } = []; + // Filter patterns are caller-supplied, and in the keystroke-driven filter box this library exists + // for, every prefix of what the user types becomes its own key. Unbounded, the caches therefore + // grow with everything ever typed for the lifetime of the process, rather than with the size of + // the data being filtered. + internal const int MaxCacheEntries = 4096; + + internal static int RegexCacheCount => RegexCache.Count; + + internal static int GlobCacheCount => GlobCache.Count; + + // Clear-and-restart rather than LRU: ConcurrentDictionary keeps no eviction order, and tracking + // one would put a write on every cache *hit* — the path the cache exists to keep cheap. This + // only ever runs on a miss, where a compile is about to happen anyway, so the Count (which does + // take the dictionary's locks) is off the hot path. At this cap a reset is rare, and costs one + // recompile per pattern still in use. + private static void AddBounded(ConcurrentDictionary cache, string cacheKey, TValue value) + { + if (cache.Count >= MaxCacheEntries) + { + cache.Clear(); + } + + cache.TryAdd(cacheKey, value); + } + // Both caches are keyed by pattern text, so the same pattern compiled at two sensitivities would // otherwise collide on the first one cached. The sensitivity is folded into the key rather than // using a tuple key, which netstandard2.0 does not get for free. @@ -379,7 +404,7 @@ private static Glob ResolveGlob(string filterToken, TextFilterCaseSensitivity ca ? Glob.Parse(filterToken, CaseInsensitiveGlobOptions) : Glob.Parse(filterToken); - GlobCache.TryAdd(cacheKey, glob); + AddBounded(GlobCache, cacheKey, glob); } return glob; @@ -423,7 +448,7 @@ public static bool DoesMatchRegex(string text, string filter, TextFilterMatchOpt regex = RegexMatchAnything(); } - RegexCache.TryAdd(cacheKey, regex); + AddBounded(RegexCache, cacheKey, regex); } Func, Func, bool> matchFunc = textFilterMatchOptions is TextFilterMatchOptions.ByWordAny