Skip to content
Merged
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
13 changes: 13 additions & 0 deletions src/EditorFeatures/Test/Utilities/PatternMatcherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,19 @@ public void TryMatchSingleWordPattern_CultureAwareSingleWordPreferCaseSensitiveE
}
}

[Fact]
public void TestCachingOfPriorResult()
{
using var matcher = PatternMatcher.CreatePatternMatcher("Goo", includeMatchedSpans: true, allowFuzzyMatching: true);
matcher.Matches("Go");

// Ensure that the above call ended up caching the result.
Assert.True(((PatternMatcher.SimplePatternMatcher)matcher).GetTestAccessor().LastCacheResultIs(areSimilar: true, candidateText: "Go"));

matcher.Matches("DefNotAMatch");
Assert.True(((PatternMatcher.SimplePatternMatcher)matcher).GetTestAccessor().LastCacheResultIs(areSimilar: false, candidateText: "DefNotAMatch"));
}

private static ImmutableArray<string> PartListToSubstrings(string identifier, in TemporaryArray<TextSpan> parts)
{
using var result = TemporaryArray<string>.Empty;
Expand Down
15 changes: 9 additions & 6 deletions src/Workspaces/Core/Portable/FindSymbols/SearchQuery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Microsoft.CodeAnalysis.FindSymbols
{
internal readonly struct SearchQuery : IDisposable
internal struct SearchQuery : IDisposable
{
/// <summary>The name being searched for. Is null in the case of custom predicate searching.. But
/// can be used for faster index based searching when it is available.</summary>
Expand All @@ -20,7 +20,10 @@ namespace Microsoft.CodeAnalysis.FindSymbols
///<summary>The predicate to fall back on if faster index searching is not possible.</summary>
private readonly Func<string, bool> _predicate;

private readonly WordSimilarityChecker? _wordSimilarityChecker;
/// <summary>
/// Not readonly as this is mutable struct.
/// </summary>
private WordSimilarityChecker _wordSimilarityChecker;

private SearchQuery(string name, SearchKind kind)
{
Expand All @@ -41,7 +44,7 @@ private SearchQuery(string name, SearchKind kind)
// once and it can cache all the information it needs while it does the AreSimilar
// check against all the possible candidates.
_wordSimilarityChecker = new WordSimilarityChecker(name, substringsAreSimilar: false);
_predicate = _wordSimilarityChecker.Value.AreSimilar;
_predicate = _wordSimilarityChecker.AreSimilar;
break;
default:
throw ExceptionUtilities.UnexpectedValue(kind);
Expand All @@ -54,8 +57,8 @@ private SearchQuery(Func<string, bool> predicate)
_predicate = predicate ?? throw new ArgumentNullException(nameof(predicate));
}

public void Dispose()
=> _wordSimilarityChecker?.Dispose();
public readonly void Dispose()
=> _wordSimilarityChecker.Dispose();

public static SearchQuery Create(string name, SearchKind kind)
=> new(name, kind);
Expand All @@ -69,7 +72,7 @@ public static SearchQuery CreateFuzzy(string name)
public static SearchQuery CreateCustom(Func<string, bool> predicate)
=> new(predicate);

public Func<string, bool> GetPredicate()
public readonly Func<string, bool> GetPredicate()
=> _predicate;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ private bool AddMatches(string container, ref TemporaryArray<PatternMatch> match
i--, j--)
{
var containerName = containerParts[j];
if (!MatchPatternSegment(containerName, _patternSegments[i], ref tempContainerMatches.AsRef(), fuzzyMatch))
if (!MatchPatternSegment(containerName, ref _patternSegments[i], ref tempContainerMatches.AsRef(), fuzzyMatch))
{
// This container didn't match the pattern piece. So there's no match at all.
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ private struct TextChunk : IDisposable
/// </summary>
public TemporaryArray<TextSpan> PatternHumps;

public readonly WordSimilarityChecker? SimilarityChecker;
/// <summary>
/// Not readonly as this value caches data within it, and so it needs to be able to mutate.
/// </summary>
public WordSimilarityChecker SimilarityChecker;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the alternative is to accept that thsi caching optimization was never working, remove it entirely, and make all this stuff readonly.


public readonly bool IsLowercase;

Expand All @@ -44,15 +47,15 @@ public TextChunk(string text, bool allowFuzzingMatching)

this.SimilarityChecker = allowFuzzingMatching
? new WordSimilarityChecker(text, substringsAreSimilar: false)
: null;
: default;

IsLowercase = !ContainsUpperCaseLetter(text);
}

public void Dispose()
{
this.PatternHumps.Dispose();
this.SimilarityChecker?.Dispose();
this.SimilarityChecker.Dispose();
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,22 @@ private static bool ContainsUpperCaseLetter(string pattern)

private PatternMatch? MatchPatternChunk(
string candidate,
in TextChunk patternChunk,
ref TextChunk patternChunk,
bool punctuationStripped,
bool fuzzyMatch)
{
return fuzzyMatch
? FuzzyMatchPatternChunk(candidate, patternChunk, punctuationStripped)
? FuzzyMatchPatternChunk(candidate, ref patternChunk, punctuationStripped)
: NonFuzzyMatchPatternChunk(candidate, patternChunk, punctuationStripped);
}

private static PatternMatch? FuzzyMatchPatternChunk(
string candidate,
in TextChunk patternChunk,
ref TextChunk patternChunk,
bool punctuationStripped)
{
Contract.ThrowIfNull(patternChunk.SimilarityChecker);
if (patternChunk.SimilarityChecker.Value.AreSimilar(candidate))
Contract.ThrowIfTrue(patternChunk.SimilarityChecker.IsDefault);
if (patternChunk.SimilarityChecker.AreSimilar(candidate))
{
return new PatternMatch(
PatternMatchKind.Fuzzy, punctuationStripped, isCaseSensitive: false, matchedSpan: null);
Expand Down Expand Up @@ -307,7 +307,7 @@ private static bool ContainsSpaceOrAsterisk(string text)
/// <returns>If there's only one match, then the return value is that match. Otherwise it is null.</returns>
private bool MatchPatternSegment(
string candidate,
in PatternSegment segment,
ref PatternSegment segment,
ref TemporaryArray<PatternMatch> matches,
bool fuzzyMatch)
{
Expand All @@ -326,7 +326,7 @@ private bool MatchPatternSegment(
if (!ContainsSpaceOrAsterisk(segment.TotalTextChunk.Text))
{
var match = MatchPatternChunk(
candidate, segment.TotalTextChunk, punctuationStripped: false, fuzzyMatch: fuzzyMatch);
candidate, ref segment.TotalTextChunk, punctuationStripped: false, fuzzyMatch: fuzzyMatch);
if (match != null)
{
matches.Add(match.Value);
Expand Down Expand Up @@ -354,7 +354,7 @@ private bool MatchPatternSegment(
if (subWordTextChunks.Length == 1)
{
var result = MatchPatternChunk(
candidate, subWordTextChunks[0], punctuationStripped: true, fuzzyMatch: fuzzyMatch);
candidate, ref subWordTextChunks[0], punctuationStripped: true, fuzzyMatch: fuzzyMatch);
if (result == null)
{
return false;
Expand All @@ -367,11 +367,11 @@ private bool MatchPatternSegment(
{
using var tempMatches = TemporaryArray<PatternMatch>.Empty;

foreach (var subWordTextChunk in subWordTextChunks)
for (int i = 0, n = subWordTextChunks.Length; i < n; i++)
{
// Try to match the candidate with this word
var result = MatchPatternChunk(
candidate, subWordTextChunk, punctuationStripped: true, fuzzyMatch: fuzzyMatch);
candidate, ref subWordTextChunks[i], punctuationStripped: true, fuzzyMatch: fuzzyMatch);
if (result == null)
return false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#nullable disable

using System;
using System.Globalization;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Collections;
Expand All @@ -12,7 +13,7 @@ namespace Microsoft.CodeAnalysis.PatternMatching
{
internal partial class PatternMatcher
{
private sealed partial class SimplePatternMatcher : PatternMatcher
internal sealed partial class SimplePatternMatcher : PatternMatcher
{
private PatternSegment _fullPatternSegment;

Expand Down Expand Up @@ -48,8 +49,17 @@ public override bool AddMatches(string candidate, ref TemporaryArray<PatternMatc
return false;
}

return MatchPatternSegment(candidate, in _fullPatternSegment, ref matches, fuzzyMatch: false) ||
MatchPatternSegment(candidate, in _fullPatternSegment, ref matches, fuzzyMatch: true);
return MatchPatternSegment(candidate, ref _fullPatternSegment, ref matches, fuzzyMatch: false) ||
MatchPatternSegment(candidate, ref _fullPatternSegment, ref matches, fuzzyMatch: true);
}

public TestAccessor GetTestAccessor()
=> new(this);

public readonly struct TestAccessor(SimplePatternMatcher simplePatternMatcher)
{
public readonly bool LastCacheResultIs(bool areSimilar, string candidateText)
=> simplePatternMatcher._fullPatternSegment.TotalTextChunk.SimilarityChecker.LastCacheResultIs(areSimilar, candidateText);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ private readonly struct CacheResult(string candidate, bool areSimilar, double si
/// </summary>
private readonly bool _substringsAreSimilar;

public readonly bool IsDefault => _source is null;

public WordSimilarityChecker(string text, bool substringsAreSimilar)
{
_source = text ?? throw new ArgumentNullException(nameof(text));
Expand All @@ -41,7 +43,12 @@ public WordSimilarityChecker(string text, bool substringsAreSimilar)
}

public readonly void Dispose()
=> _editDistance.Dispose();
{
if (this.IsDefault)
return;

_editDistance.Dispose();
}

public static bool AreSimilar(string originalText, string candidateText)
=> AreSimilar(originalText, candidateText, substringsAreSimilar: false);
Expand Down Expand Up @@ -156,5 +163,8 @@ private static double Penalty(string candidateText, string originalText)

return 0;
}

public readonly bool LastCacheResultIs(bool areSimilar, string candidateText)
=> _lastAreSimilarResult.AreSimilar == areSimilar && _lastAreSimilarResult.CandidateText == candidateText;
}
}