Skip to content

Commit

Permalink
fix: improve member matching performance for member names with lots o…
Browse files Browse the repository at this point in the history
…f upper case letters (#1447)

* Only compute member name permutations until a match is found
* Limit number of computed member permutations
  • Loading branch information
latonz authored Aug 13, 2024
1 parent 531de06 commit 24fc65d
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 32 deletions.
9 changes: 9 additions & 0 deletions docs/docs/breaking-changes/4-0.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ description: How to upgrade to Mapperly v4.0 and a list of all its breaking chan
- To account for changed diagnostics adjust your `.editorconfig` as needed, see [updated diagnostics](#updated-diagnostics).
- The ordering of the member assignments in mappings may change, if you rely on the order of members being mapped, you may need to diff and verify the generated source code.
- Well-known .NET immutable types are not copied, even if `UseDeepCloning` is enabled.
- For long property names, auto-flattening may not work anymore and may need to be configured manually by applying the `MapPropertyAttribute`.

## MapPropertyAttribute constructors

Expand Down Expand Up @@ -49,6 +50,14 @@ This may break existing code because a member that wasn't mapped before may now
New diagnostics can also be reported for existing mappings
(e.g., if a new member is being mapped, but Mapperly cannot create a conversion for its types).

## Limited auto-flattening

Mapperly tries to flatten properties automatically.
For this Mapperly tries to access object members based on the pascal case member name notation of C#.
Starting with 4.0 Mapperly will only try up to 256 member path permutations.
Before all possible permutations were tried.
Auto-flattening can always be overwritten by applying the `MapPropertyAttribute`.

## Updated diagnostics

The following diagnostics are changed, removed or replaced.
Expand Down
7 changes: 6 additions & 1 deletion src/Riok.Mapperly/Configuration/StringMemberPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@

namespace Riok.Mapperly.Configuration;

public record StringMemberPath(ImmutableEquatableArray<string> Path)
public readonly record struct StringMemberPath(ImmutableEquatableArray<string> Path)
{
public static readonly StringMemberPath Empty = new(ImmutableEquatableArray<string>.Empty);

public StringMemberPath(IEnumerable<string> path)
: this(path.ToImmutableEquatableArray()) { }

public const char MemberAccessSeparator = '.';
private const string MemberAccessSeparatorString = ".";

public string FullName => string.Join(MemberAccessSeparatorString, Path);

public override string ToString() => FullName;

public StringMemberPath SkipRoot() => new(Path.Skip(1).ToImmutableEquatableArray());
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ protected bool TryGetMemberValueConfigs(
) => _state.TryGetMemberValueConfigs(targetMemberName, ignoreCase, out memberConfigs);

protected virtual bool TryFindSourcePath(
IReadOnlyList<IReadOnlyList<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
bool ignoreCase,
[NotNullWhen(true)] out SourceMemberPath? sourcePath
)
Expand Down Expand Up @@ -183,7 +183,7 @@ private IEnumerable<MemberMappingInfo> ResolveMemberMappingInfo(IEnumerable<Memb
private MemberMappingInfo? ResolveMemberMappingInfo(MemberValueMappingConfiguration config)
{
if (
!BuilderContext.SymbolAccessor.TryFindMemberPath(Mapping.TargetType, config.Target.Path, out var foundMemberPath)
!BuilderContext.SymbolAccessor.TryFindMemberPath(Mapping.TargetType, config.Target, out var foundMemberPath)
|| foundMemberPath is not NonEmptyMemberPath targetMemberPath
)
{
Expand All @@ -205,7 +205,7 @@ private IEnumerable<MemberMappingInfo> ResolveMemberMappingInfo(IEnumerable<Memb
private MemberMappingInfo? ResolveMemberMappingInfo(MemberMappingConfiguration config)
{
if (
!BuilderContext.SymbolAccessor.TryFindMemberPath(Mapping.TargetType, config.Target.Path, out var foundMemberPath)
!BuilderContext.SymbolAccessor.TryFindMemberPath(Mapping.TargetType, config.Target, out var foundMemberPath)
|| foundMemberPath is not NonEmptyMemberPath targetMemberPath
)
{
Expand All @@ -229,7 +229,7 @@ private IEnumerable<MemberMappingInfo> ResolveMemberMappingInfo(IEnumerable<Memb

private bool ResolveMemberConfigSourcePath(MemberMappingConfiguration config, [NotNullWhen(true)] out SourceMemberPath? sourcePath)
{
if (!BuilderContext.SymbolAccessor.TryFindMemberPath(Mapping.SourceType, config.Source.Path, out var sourceMemberPath))
if (!BuilderContext.SymbolAccessor.TryFindMemberPath(Mapping.SourceType, config.Source, out var sourceMemberPath))
{
BuilderContext.ReportDiagnostic(
DiagnosticDescriptors.ConfiguredMappingSourceMemberNotFound,
Expand All @@ -255,7 +255,7 @@ private bool TryFindSourcePath(
)
{
ignoreCase ??= BuilderContext.Configuration.Mapper.PropertyNameMappingStrategy == PropertyNameMappingStrategy.CaseInsensitive;
var pathCandidates = MemberPathCandidateBuilder.BuildMemberPathCandidates(targetMemberName).Select(cs => cs.ToList()).ToList();
var pathCandidates = MemberPathCandidateBuilder.BuildMemberPathCandidates(targetMemberName);

// First, try to find the property on (a sub-path of) the source type itself. (If this is undesired, an Ignore property can be used.)
if (TryFindSourcePath(pathCandidates, ignoreCase.Value, out sourceMemberPath))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Diagnostics.CodeAnalysis;
using Riok.Mapperly.Configuration;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Symbols.Members;

Expand Down Expand Up @@ -28,7 +29,7 @@ private static List<MemberPath> ResolveNestedMappings(MappingBuilderContext ctx)

foreach (var nestedMemberConfig in ctx.Configuration.Members.NestedMappings)
{
if (!ctx.SymbolAccessor.TryFindMemberPath(ctx.Source, nestedMemberConfig.Source.Path, out var memberPath))
if (!ctx.SymbolAccessor.TryFindMemberPath(ctx.Source, nestedMemberConfig.Source, out var memberPath))
{
ctx.ReportDiagnostic(
DiagnosticDescriptors.ConfiguredMappingNestedMemberNotFound,
Expand All @@ -45,7 +46,7 @@ private static List<MemberPath> ResolveNestedMappings(MappingBuilderContext ctx)
}

public bool TryFindNestedSourcePath(
List<List<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
bool ignoreCase,
[NotNullWhen(true)] out SourceMemberPath? sourceMemberPath
)
Expand All @@ -61,7 +62,7 @@ public bool TryFindNestedSourcePath(
}

private bool TryFindNestedSourcePath(
List<List<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
bool ignoreCase,
MemberPath nestedMemberPath,
[NotNullWhen(true)] out SourceMemberPath? sourceMemberPath
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Configuration;
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;
using Riok.Mapperly.Symbols.Members;
Expand Down Expand Up @@ -54,7 +55,7 @@ public void AddTupleConstructorParameterMapping(ValueTupleConstructorParameterMa
}

protected override bool TryFindSourcePath(
IReadOnlyList<IReadOnlyList<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
bool ignoreCase,
[NotNullWhen(true)] out SourceMemberPath? sourceMemberPath
)
Expand All @@ -69,16 +70,16 @@ protected override bool TryFindSourcePath(
}

private bool TryFindSecondaryTupleSourceField(
IReadOnlyList<IReadOnlyList<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
[NotNullWhen(true)] out SourceMemberPath? sourcePath
)
{
foreach (var pathParts in pathCandidates)
{
if (pathParts.Count != 1)
if (pathParts.Path.Count != 1)
continue;

var name = pathParts[0];
var name = pathParts.Path[0];
if (_secondarySourceNames.TryGetValue(name, out var sourceField))
{
var sourceFieldMember = new FieldMember(sourceField, BuilderContext.SymbolAccessor);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Configuration;
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;
using Riok.Mapperly.Symbols.Members;
Expand Down Expand Up @@ -54,7 +55,7 @@ public void AddTupleConstructorParameterMapping(ValueTupleConstructorParameterMa
}

protected override bool TryFindSourcePath(
IReadOnlyList<IReadOnlyList<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
bool ignoreCase,
[NotNullWhen(true)] out SourceMemberPath? sourceMemberPath
)
Expand All @@ -69,16 +70,16 @@ protected override bool TryFindSourcePath(
}

private bool TryFindSecondaryTupleSourceField(
IReadOnlyList<IReadOnlyList<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
[NotNullWhen(true)] out SourceMemberPath? sourcePath
)
{
foreach (var pathParts in pathCandidates)
{
if (pathParts.Count != 1)
if (pathParts.Path.Count != 1)
continue;

var name = pathParts[0];
var name = pathParts.Path[0];
if (_secondarySourceNames.TryGetValue(name, out var sourceField))
{
var sourceFieldMember = new FieldMember(sourceField, BuilderContext.SymbolAccessor);
Expand Down
20 changes: 15 additions & 5 deletions src/Riok.Mapperly/Descriptors/MemberPathCandidateBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,32 +1,42 @@
using Riok.Mapperly.Configuration;

namespace Riok.Mapperly.Descriptors;

public static class MemberPathCandidateBuilder
{
/// <summary>
/// Maximum number of indices which are considered to compute the member path candidates.
/// </summary>
private const int MaxPascalCaseIndices = 8;

/// <summary>
/// Splits a name into pascal case chunks and joins them together in all possible combinations.
/// <example><c>"MyValueId"</c> leads to <c>[["MyValueId"], ["My", "ValueId"], ["MyValue", "Id"], ["My", "Value", "Id"]</c></example>
/// </summary>
/// <param name="name">The name to build candidates from.</param>
/// <returns>The joined member path groups.</returns>
public static IEnumerable<IEnumerable<string>> BuildMemberPathCandidates(string name)
public static IEnumerable<StringMemberPath> BuildMemberPathCandidates(string name)
{
if (name.Length == 0)
yield break;

// yield full string
yield return new[] { name };
// as a fast path (often member match by their exact name)
yield return new StringMemberPath([name]);

var indices = GetPascalCaseSplitIndices(name).ToArray();
var indices = GetPascalCaseSplitIndices(name).Take(MaxPascalCaseIndices).ToArray();
if (indices.Length == 0)
yield break;

// try all permutations, skipping the first because the full string is already yielded
var permutationsCount = 1 << indices.Length;
for (var i = 1; i < permutationsCount; i++)
{
yield return BuildName(name, indices, i);
yield return new StringMemberPath(BuildPermutationParts(name, indices, i));
}
}

private static IEnumerable<string> BuildName(string source, int[] splitIndices, int enabledSplitPositions)
private static IEnumerable<string> BuildPermutationParts(string source, int[] splitIndices, int enabledSplitPositions)
{
var lastSplitIndex = 0;
var currentSplitPosition = 1;
Expand Down
17 changes: 9 additions & 8 deletions src/Riok.Mapperly/Descriptors/SymbolAccessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Configuration;
using Riok.Mapperly.Helpers;
using Riok.Mapperly.Symbols;
using Riok.Mapperly.Symbols.Members;
Expand Down Expand Up @@ -254,20 +255,20 @@ internal IReadOnlyCollection<IMappableMember> GetAllAccessibleMappableMembers(IT

internal bool TryFindMemberPath(
IReadOnlyDictionary<string, IMappableMember> members,
IEnumerable<IReadOnlyList<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
bool ignoreCase,
[NotNullWhen(true)] out MemberPath? memberPath
)
{
var foundPath = new List<IMappableMember>();
foreach (var pathCandidate in pathCandidates)
{
if (!members.TryGetValue(pathCandidate[0], out var member))
if (!members.TryGetValue(pathCandidate.Path[0], out var member))
continue;

foundPath.Clear();
foundPath.Add(member);
if (pathCandidate.Count == 1 || TryFindPath(member.Type, pathCandidate.Skip(1), ignoreCase, foundPath))
if (pathCandidate.Path.Count == 1 || TryFindPath(member.Type, pathCandidate.SkipRoot(), ignoreCase, foundPath))
{
memberPath = new NonEmptyMemberPath(member.Type, foundPath);
return true;
Expand All @@ -280,7 +281,7 @@ internal bool TryFindMemberPath(

internal bool TryFindMemberPath(
ITypeSymbol type,
IEnumerable<IReadOnlyList<string>> pathCandidates,
IEnumerable<StringMemberPath> pathCandidates,
IReadOnlyCollection<string> ignoredNames,
bool ignoreCase,
[NotNullWhen(true)] out MemberPath? memberPath
Expand All @@ -290,7 +291,7 @@ internal bool TryFindMemberPath(
foreach (var pathCandidate in pathCandidates)
{
// fast path for exact case matches
if (ignoredNames.Contains(pathCandidate[0]))
if (ignoredNames.Contains(pathCandidate.Path[0]))
continue;

// reuse List instead of allocating a new one
Expand All @@ -310,7 +311,7 @@ internal bool TryFindMemberPath(
return false;
}

internal bool TryFindMemberPath(ITypeSymbol type, IReadOnlyCollection<string> path, [NotNullWhen(true)] out MemberPath? memberPath)
internal bool TryFindMemberPath(ITypeSymbol type, StringMemberPath path, [NotNullWhen(true)] out MemberPath? memberPath)
{
var foundPath = new List<IMappableMember>();
if (TryFindPath(type, path, false, foundPath))
Expand All @@ -323,9 +324,9 @@ internal bool TryFindMemberPath(ITypeSymbol type, IReadOnlyCollection<string> pa
return false;
}

private bool TryFindPath(ITypeSymbol type, IEnumerable<string> path, bool ignoreCase, ICollection<IMappableMember> foundPath)
private bool TryFindPath(ITypeSymbol type, StringMemberPath path, bool ignoreCase, ICollection<IMappableMember> foundPath)
{
foreach (var name in path)
foreach (var name in path.Path)
{
// get T if type is Nullable<T>, prevents Value being treated as a member
var actualType = type.NonNullableValueType() ?? type;
Expand Down
10 changes: 9 additions & 1 deletion src/Riok.Mapperly/Helpers/ImmutableEquatableArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Runtime.CompilerServices;

namespace Riok.Mapperly.Helpers;

/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
/// </summary>
[CollectionBuilder(typeof(ImmutableEquatableArray), nameof(ImmutableEquatableArray.Create))]
public sealed class ImmutableEquatableArray<T> : IEquatable<ImmutableEquatableArray<T>>, IReadOnlyList<T>
where T : IEquatable<T>
{
Expand All @@ -21,6 +23,9 @@ public sealed class ImmutableEquatableArray<T> : IEquatable<ImmutableEquatableAr
public ImmutableEquatableArray(IEnumerable<T> values)
: this(values.ToArray()) { }

public ImmutableEquatableArray(ReadOnlySpan<T> values)
: this(values.ToArray()) { }

/// <summary>
/// Initializes a new <see cref="ImmutableEquatableArray{T}"/> instance.
/// This constructor should only be called with arrays
Expand All @@ -33,7 +38,7 @@ private ImmutableEquatableArray(T[] values)

public static ImmutableEquatableArray<T> Empty => _empty ??= new([]);

public bool Equals(ImmutableEquatableArray<T>? other) => other != null && ((ReadOnlySpan<T>)_values).SequenceEqual(other._values);
public bool Equals(ImmutableEquatableArray<T> other) => _values.SequenceEqual(other._values);

public override bool Equals(object? obj) => obj is ImmutableEquatableArray<T> other && Equals(other);

Expand Down Expand Up @@ -84,4 +89,7 @@ public static class ImmutableEquatableArray
{
public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values)
where T : IEquatable<T> => new(values);

public static ImmutableEquatableArray<T> Create<T>(ReadOnlySpan<T> values)
where T : IEquatable<T> => new(values);
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,14 @@ public void BuildMemberPathCandidatesShouldWork(string name, string[] chunks)
{
MemberPathCandidateBuilder
.BuildMemberPathCandidates(name)
.Select(x => string.Join(".", x))
.Select(x => x.FullName)
.Should()
.BeEquivalentTo(chunks, o => o.WithStrictOrdering());
}

[Fact]
public void BuildMemberPathCandidatesWithPascalCaseShouldLimitPermutations()
{
MemberPathCandidateBuilder.BuildMemberPathCandidates("NOT_A_PASCAL_CASE_STRING").Should().HaveCount(256);
}
}

0 comments on commit 24fc65d

Please sign in to comment.