Skip to content

Avoid per-key allocations in GetProperty #1121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lahma wants to merge 1 commit into
base: master
Choose a base branch
from
+46 −16
Open

Avoid per-key allocations in GetProperty #1121

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 46 additions & 16 deletions YamlDotNet/Serialization/TypeInspectors/TypeInspectorSkeleton.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,49 @@ public abstract class TypeInspectorSkeleton : ITypeInspector

public IPropertyDescriptor GetProperty(Type type, object? container, string name, [MaybeNullWhen(true)] bool ignoreUnmatched, bool caseInsensitivePropertyMatching)
{
IEnumerable<IPropertyDescriptor> candidates;
// This runs once per YAML key during deserialization, so avoid the per-call LINQ closure
// and iterator that .Where(p => p.Name == name) allocated. When the property list is an
// IReadOnlyList (it is when it comes from CachedTypeInspector) the match scan is
// allocation-free. Semantics are unchanged: no match, single match, and ambiguous match
// are handled exactly as before.
var comparison = caseInsensitivePropertyMatching ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
var properties = GetProperties(type, container);

if (caseInsensitivePropertyMatching)
IPropertyDescriptor? match = null;

if (properties is IReadOnlyList<IPropertyDescriptor> list)
{
candidates = GetProperties(type, container)
.Where(p => p.Name.Equals(name, StringComparison.OrdinalIgnoreCase));
for (var i = 0; i < list.Count; i++)
{
var candidate = list[i];
if (candidate.Name.Equals(name, comparison))
{
if (match != null)
{
throw MultipleMatchesException(type, container, name, comparison);
}

match = candidate;
}
}
}
else
{
candidates = GetProperties(type, container)
.Where(p => p.Name == name);
foreach (var candidate in properties)
{
if (candidate.Name.Equals(name, comparison))
{
if (match != null)
{
throw MultipleMatchesException(type, container, name, comparison);
}

match = candidate;
}
}
}

using var enumerator = candidates.GetEnumerator();
if (!enumerator.MoveNext())
if (match == null)
{
if (ignoreUnmatched)
{
Expand All @@ -62,16 +90,18 @@ public IPropertyDescriptor GetProperty(Type type, object? container, string name
throw new SerializationException($"Property '{name}' not found on type '{type.FullName}'.");
}

var property = enumerator.Current;
return match;
}

if (enumerator.MoveNext())
{
throw new SerializationException(
$"Multiple properties with the name/alias '{name}' already exists on type '{type.FullName}', maybe you're misusing YamlAlias or maybe you are using the wrong naming convention? The matching properties are: {string.Join(", ", candidates.Select(p => p.Name).ToArray())}"
);
}
private SerializationException MultipleMatchesException(Type type, object? container, string name, StringComparison comparison)
{
var matches = GetProperties(type, container)
.Where(p => p.Name.Equals(name, comparison))
.Select(p => p.Name);

return property;
return new SerializationException(
$"Multiple properties with the name/alias '{name}' already exists on type '{type.FullName}', maybe you're misusing YamlAlias or maybe you are using the wrong naming convention? The matching properties are: {string.Join(", ", matches.ToArray())}"
);
}

public abstract bool HasParseMethod(Type type);
Expand Down
Loading