Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions TextFilter.Test/TextFilterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
62 changes: 38 additions & 24 deletions TextFilter/TextFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,11 @@
// 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
Expand Down Expand Up @@ -300,7 +303,7 @@

return ExcludedTokenPrefixes.Contains(prefix)
? TextFilterTokenType.Excluded
: RequiredTokenPrefixes.Contains(prefix)

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.

Check warning on line 306 in TextFilter/TextFilter.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Extract this nested ternary operation into an independent statement.
? TextFilterTokenType.Required
: TextFilterTokenType.Optional;
})
Expand Down Expand Up @@ -694,29 +697,7 @@
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);
}

Expand Down Expand Up @@ -746,6 +727,39 @@
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
Expand Down
Loading