From ca6b5328b9d2d53e54489d9aaab2a868f2b21ce2 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Sat, 31 Oct 2026 17:24:45 +0000 Subject: [PATCH] Build filter regexes interpreted instead of compiled [patch] A filter box makes a new pattern on every keystroke and matches it once, so RegexOptions.Compiled cost far more than it saved: about ten times the per-keystroke work and several times the memory of a full cache. The invalid-pattern fallback is now one shared, interpreted instance with the match timeout, instead of a newly compiled ".*" on every call. Fixes ktsu-dev/TextFilter#144 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW2nwQnKNnZqSnbuUxe --- TextFilter.Test/TextFilterTests.cs | 38 ++++++++++++++++++ TextFilter/TextFilter.cs | 62 ++++++++++++++++++------------ 2 files changed, 76 insertions(+), 24 deletions(-) diff --git a/TextFilter.Test/TextFilterTests.cs b/TextFilter.Test/TextFilterTests.cs index 3250a64..f91692b 100644 --- a/TextFilter.Test/TextFilterTests.cs +++ b/TextFilter.Test/TextFilterTests.cs @@ -897,6 +897,44 @@ public void TheGlobCacheStaysBoundedAsDistinctPatternsArrive() "The glob cache should be bounded, not grow with every pattern ever seen."); } + [TestMethod] + [DataRow(TextFilterCaseSensitivity.CaseSensitive)] + [DataRow(TextFilterCaseSensitivity.CaseInsensitive)] + public void FilterRegexesAreInterpretedNotCompiled(TextFilterCaseSensitivity caseSensitivity) + { + // A filter box makes a new pattern on every keystroke and matches it once, so compiling each + // one to IL costs far more than it saves (#144). + System.Text.RegularExpressions.Regex regex = TextFilter.CreateFilterRegex(@"Module1\d+/File\w*\.cs", caseSensitivity); + + Assert.AreEqual( + System.Text.RegularExpressions.RegexOptions.None, + regex.Options & System.Text.RegularExpressions.RegexOptions.Compiled, + "A per-keystroke filter pattern should not be compiled to IL."); + Assert.AreEqual( + System.Text.RegularExpressions.RegexOptions.CultureInvariant, + regex.Options & System.Text.RegularExpressions.RegexOptions.CultureInvariant, + "Case folding must stay culture-invariant."); + Assert.AreEqual( + caseSensitivity is TextFilterCaseSensitivity.CaseInsensitive, + regex.Options.HasFlag(System.Text.RegularExpressions.RegexOptions.IgnoreCase)); + Assert.AreEqual(System.TimeSpan.FromSeconds(1), regex.MatchTimeout, "The match timeout must still apply."); + } + + [TestMethod] + public void AnInvalidRegexFilterSharesOneInterpretedMatchAnythingInstance() + { + // An invalid pattern, such as "[" while it is still being typed, used to build and compile a + // new ".*" each time. It now reuses one instance, which is interpreted and bounded by the timeout. + System.Text.RegularExpressions.Regex first = TextFilter.CreateFilterRegex("[", TextFilterCaseSensitivity.CaseSensitive); + System.Text.RegularExpressions.Regex second = TextFilter.CreateFilterRegex("(unclosed", TextFilterCaseSensitivity.CaseInsensitive); + + Assert.AreSame(first, second, "Every invalid pattern should fall back to the same cached instance."); + Assert.AreEqual(System.Text.RegularExpressions.RegexOptions.None, first.Options & System.Text.RegularExpressions.RegexOptions.Compiled); + Assert.AreEqual(System.TimeSpan.FromSeconds(1), first.MatchTimeout); + Assert.IsTrue(first.IsMatch("anything at all"), "The fallback should still match anything."); + Assert.IsTrue(TextFilter.DoesMatchRegex("hello world", "[", TextFilterMatchOptions.ByWordAny)); + } + [TestMethod] public void ACachedPatternStillMatchesAfterTheCacheHasReset() { diff --git a/TextFilter/TextFilter.cs b/TextFilter/TextFilter.cs index 927c00c..b37f096 100644 --- a/TextFilter/TextFilter.cs +++ b/TextFilter/TextFilter.cs @@ -124,8 +124,11 @@ private static string CacheKey(string pattern, TextFilterCaseSensitivity caseSen // filter needs and short enough that a pathological one cannot wedge a UI. private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(1); + // Stands in for a pattern that does not parse, such as "[" half-way through being typed. One + // shared instance, like RegexMatchNothing below, so an invalid pattern costs a cache entry and + // nothing more, rather than a fresh regex on every keystroke that produces one. [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "SYSLIB1045:Convert to 'GeneratedRegexAttribute'.", Justification = "Not available in older frameworks")] - private static Regex RegexMatchAnything() => new(".*", RegexOptions.Compiled); + private static readonly Regex RegexMatchAnything = new(".*", RegexOptions.None, RegexMatchTimeout); // Stands in for a pattern that has already run out of time once. The timeout bounds a single // IsMatch call, so without this the same pathological pattern would pay it again for every word @@ -694,29 +697,7 @@ public static bool DoesMatchRegex(string text, string filter, TextFilterMatchOpt string cacheKey = CacheKey(filter, caseSensitivity); if (!RegexCache.TryGetValue(cacheKey, out Regex? regex)) { - // CultureInvariant, because IgnoreCase alone folds case using the thread's CurrentCulture. - // Under tr-TR that stops "i" and "I" being the same letter, so a filter of "img" no longer - // matches "IMG_1234.JPG" -- and the cache below is keyed by pattern and sensitivity only, - // so whichever culture happened to compile the pattern first decides the answer for every - // later caller on any thread. A filter pattern is machine text, not prose, and the glob - // path already folds invariantly, so the two paths now agree. It applies under CaseSensitive - // too, because the pattern itself can ask for a fold with an inline (?i), and that fold is - // just as culture-dependent and just as cached as the one the option asks for. - RegexOptions regexOptions = caseSensitivity is TextFilterCaseSensitivity.CaseInsensitive - ? RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.CultureInvariant - : RegexOptions.Compiled | RegexOptions.CultureInvariant; - - try - { - regex = new Regex(filter, regexOptions, RegexMatchTimeout); - } - catch (ArgumentException) - { - // invalid regex pattern - // match anything if the pattern is invalid and cache it so that we don't trigger the exception again - regex = RegexMatchAnything(); - } - + regex = CreateFilterRegex(filter, caseSensitivity); AddBounded(RegexCache, cacheKey, regex); } @@ -746,6 +727,39 @@ public static bool DoesMatchRegex(string text, string filter, TextFilterMatchOpt return matchFunc(textTokens, textToken => IsMatchWithinTimeout(ref activeRegex, cacheKey, textToken)); } + // Builds the regex DoesMatchRegex caches for a filter pattern. + // + // Interpreted rather than RegexOptions.Compiled. A filter box produces a new pattern on every + // keystroke ("M", "Mo", "Mod", ...), and each one is matched against the visible items once and + // then never again. Compiling to IL only pays off over many matches, so for these patterns it + // made each keystroke about ten times slower and each cached entry several times larger, and on + // .NET 7 and later the interpreter is fast for inputs as short as item names. + internal static Regex CreateFilterRegex(string filter, TextFilterCaseSensitivity caseSensitivity) + { + // CultureInvariant, because IgnoreCase alone folds case using the thread's CurrentCulture. + // Under tr-TR that stops "i" and "I" being the same letter, so a filter of "img" no longer + // matches "IMG_1234.JPG" -- and the cache in DoesMatchRegex is keyed by pattern and sensitivity + // only, so whichever culture happened to compile the pattern first decides the answer for every + // later caller on any thread. A filter pattern is machine text, not prose, and the glob + // path already folds invariantly, so the two paths now agree. It applies under CaseSensitive + // too, because the pattern itself can ask for a fold with an inline (?i), and that fold is + // just as culture-dependent and just as cached as the one the option asks for. + RegexOptions regexOptions = caseSensitivity is TextFilterCaseSensitivity.CaseInsensitive + ? RegexOptions.IgnoreCase | RegexOptions.CultureInvariant + : RegexOptions.CultureInvariant; + + try + { + return new Regex(filter, regexOptions, RegexMatchTimeout); + } + catch (ArgumentException) + { + // An invalid pattern matches anything, and DoesMatchRegex caches the result so the + // exception is not triggered again. + return RegexMatchAnything; + } + } + private static bool IsMatchWithinTimeout(ref Regex regex, string cacheKey, string input) { try