Skip to content
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.

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

Reduce allocations in TokenSegment.TryMatch #5933

Merged
merged 3 commits into from
Jul 31, 2024
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
89 changes: 0 additions & 89 deletions src/NuGet.Core/NuGet.Packaging/ArrayPool.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,76 +15,40 @@ public class ContentPropertyDefinition
{
private static readonly Func<object, object, bool> EqualsTest = (left, right) => Equals(left, right);

public ContentPropertyDefinition(string name)
: this(name, null, null, null, null, false)
{
}

public ContentPropertyDefinition(
internal ContentPropertyDefinition(
zivkan marked this conversation as resolved.
Show resolved Hide resolved
string name,
Func<string, PatternTable, object> parser)
Func<ReadOnlyMemory<char>, PatternTable, object> parser)
: this(name, parser, null, null, null, false)
{
}

public ContentPropertyDefinition(
internal ContentPropertyDefinition(
string name,
Func<object, object, bool> compatibilityTest)
: this(name, null, compatibilityTest, null, null, false)
{
}

public ContentPropertyDefinition(
string name,
Func<string, PatternTable, object> parser,
Func<ReadOnlyMemory<char>, PatternTable, object> parser,
Func<object, object, bool> compatibilityTest)
: this(name, parser, compatibilityTest, null, null, false)
{
}

public ContentPropertyDefinition(string name,
Func<string, PatternTable, object> parser,
internal ContentPropertyDefinition(string name,
Func<ReadOnlyMemory<char>, PatternTable, object> parser,
Func<object, object, bool> compatibilityTest,
Func<object, object, object, int> compareTest)
: this(name, parser, compatibilityTest, compareTest, null, false)
{
}

public ContentPropertyDefinition(
string name,
IEnumerable<string> fileExtensions)
: this(name, null, null, null, fileExtensions, false)
{
}

public ContentPropertyDefinition(
internal ContentPropertyDefinition(
string name,
Func<string, PatternTable, object> parser,
Func<ReadOnlyMemory<char>, PatternTable, object> parser,
IEnumerable<string> fileExtensions)
: this(name, parser, null, null, fileExtensions, false)
{
}

public ContentPropertyDefinition(
string name,
IEnumerable<string> fileExtensions,
bool allowSubfolders)
: this(name, null, null, null, fileExtensions, allowSubfolders)
{
}

public ContentPropertyDefinition(
string name,
Func<string, PatternTable, object> parser,
IEnumerable<string> fileExtensions,
bool allowSubfolders)
: this(name, parser, null, null, fileExtensions, allowSubfolders)
{
}

public ContentPropertyDefinition(
internal ContentPropertyDefinition(
string name,
Func<string, PatternTable, object> parser,
Func<ReadOnlyMemory<char>, PatternTable, object> parser,
Func<object, object, bool> compatibilityTest,
Func<object, object, object, int> compareTest,
IEnumerable<string> fileExtensions,
Expand All @@ -104,11 +68,11 @@ public ContentPropertyDefinition(

public bool FileExtensionAllowSubFolders { get; }

public Func<string, PatternTable, object> Parser { get; }
internal Func<ReadOnlyMemory<char>, PatternTable, object> Parser { get; }

public virtual bool TryLookup(string name, PatternTable table, out object value)
internal virtual bool TryLookup(ReadOnlyMemory<char> name, PatternTable table, out object value)
{
if (name == null)
if (name.IsEmpty)
{
value = null;
return false;
Expand All @@ -120,9 +84,9 @@ public virtual bool TryLookup(string name, PatternTable table, out object value)
{
foreach (var fileExtension in FileExtensions)
{
if (name.EndsWith(fileExtension, StringComparison.OrdinalIgnoreCase))
if (name.Span.EndsWith(fileExtension.AsSpan(), StringComparison.OrdinalIgnoreCase))
{
value = name;
value = name.ToString();
return true;
}
}
Expand All @@ -142,10 +106,10 @@ public virtual bool TryLookup(string name, PatternTable table, out object value)
return false;
}

private static bool ContainsSlash(string name)
private static bool ContainsSlash(ReadOnlyMemory<char> name)
{
var containsSlash = false;
foreach (var ch in name)
foreach (var ch in name.Span)
{
if (ch == '/' || ch == '\\')
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ internal override bool TryMatch(
{
break;
}
var substring = path.Substring(startIndex, delimiterIndex - startIndex);
ReadOnlyMemory<char> substring = path.AsMemory(startIndex, delimiterIndex - startIndex);
object value;
if (propertyDefinition.TryLookup(substring, _table, out value))
{
Expand All @@ -206,7 +206,7 @@ internal override bool TryMatch(
}
if (StringComparer.Ordinal.Equals(_token, "tfm"))
{
item.Properties.Add("tfm_raw", substring);
item.Properties.Add("tfm_raw", substring.ToString());
}
item.Properties.Add(_token, value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using NuGet.ContentModel;
using NuGet.Frameworks;
using NuGet.Packaging.Core;
Expand All @@ -22,7 +21,7 @@ public class ManagedCodeConventions

private static readonly ContentPropertyDefinition AnyProperty = new ContentPropertyDefinition(
PropertyNames.AnyValue,
parser: (o, t) => o); // Identity parser, all strings are valid for any
parser: (o, t) => o.ToString()); // Identity parser, all strings are valid for any
private static readonly ContentPropertyDefinition AssemblyProperty = new ContentPropertyDefinition(PropertyNames.ManagedAssembly,
parser: AllowEmptyFolderParser,
fileExtensions: new[] { ".dll", ".winmd", ".exe" });
Expand Down Expand Up @@ -69,8 +68,7 @@ public class ManagedCodeConventions

private RuntimeGraph _runtimeGraph;

private Dictionary<string, NuGetFramework> _frameworkCache
= new Dictionary<string, NuGetFramework>(StringComparer.Ordinal);
private Dictionary<ReadOnlyMemory<char>, NuGetFramework> _frameworkCache = new(ReadOnlyMemoryCharComparerOrdinal.Instance);

public ManagedCodeCriteria Criteria { get; }
public IReadOnlyDictionary<string, ContentPropertyDefinition> Properties { get; }
Expand All @@ -90,7 +88,7 @@ public ManagedCodeConventions(RuntimeGraph runtimeGraph)

props[PropertyNames.RuntimeIdentifier] = new ContentPropertyDefinition(
PropertyNames.RuntimeIdentifier,
parser: (o, t) => o, // Identity parser, all strings are valid runtime ids :)
parser: (o, t) => o.ToString(), // Identity parser, all strings are valid runtime ids
compatibilityTest: RuntimeIdentifier_CompatibilityTest);

props[PropertyNames.TargetFrameworkMoniker] = new ContentPropertyDefinition(
Expand Down Expand Up @@ -125,7 +123,7 @@ private bool RuntimeIdentifier_CompatibilityTest(object criteria, object availab
}
}

private static object CodeLanguage_Parser(string name, PatternTable table)
private static object CodeLanguage_Parser(ReadOnlyMemory<char> name, PatternTable table)
{
if (table != null)
{
Expand All @@ -138,17 +136,17 @@ private static object CodeLanguage_Parser(string name, PatternTable table)

// Code language values must be alpha numeric.
// PERF: use foreach to avoid CharEnumerator allocation
foreach (char c in name)
foreach (char c in name.Span)
{
if (!char.IsLetterOrDigit(c))
{
return null;
}
}
return name;
return name.ToString();
}

private static object Locale_Parser(string name, PatternTable table)
private static object Locale_Parser(ReadOnlyMemory<char> name, PatternTable table)
{
if (table != null)
{
Expand All @@ -161,18 +159,18 @@ private static object Locale_Parser(string name, PatternTable table)

if (name.Length == 2)
{
return name;
return name.ToString();
}
else if (name.Length >= 4 && name[2] == '-')
else if (name.Length >= 4 && name.Span[2] == '-')
{
return name;
return name.ToString();
}

return null;
}

private object TargetFrameworkName_Parser(
string name,
ReadOnlyMemory<char> name,
PatternTable table)
{
object obj = null;
Expand All @@ -187,21 +185,21 @@ private object TargetFrameworkName_Parser(
}

// Check the cache for an exact match
if (!string.IsNullOrEmpty(name))
if (!name.IsEmpty)
{
NuGetFramework cachedResult;
if (!_frameworkCache.TryGetValue(name, out cachedResult))
{
// Parse and add the framework to the cache
cachedResult = TargetFrameworkName_ParserCore(name);
cachedResult = TargetFrameworkName_ParserCore(name.ToString());
_frameworkCache.Add(name, cachedResult);
}

return cachedResult;
}

// Let the framework parser handle null/empty and create the error message.
return TargetFrameworkName_ParserCore(name);
return TargetFrameworkName_ParserCore(name.ToString());
}

private static NuGetFramework TargetFrameworkName_ParserCore(string name)
Expand All @@ -226,10 +224,15 @@ private static NuGetFramework TargetFrameworkName_ParserCore(string name)
return new NuGetFramework(name, FrameworkConstants.EmptyVersion);
}

private static object AllowEmptyFolderParser(string s, PatternTable table)
private static object AllowEmptyFolderParser(ReadOnlyMemory<char> s, PatternTable table)
{
// Accept "_._" as a pseudo-assembly
return PackagingCoreConstants.EmptyFolder.Equals(s, StringComparison.Ordinal) ? s : null;
if (MemoryExtensions.Equals(PackagingCoreConstants.EmptyFolder.AsSpan(), s.Span, StringComparison.Ordinal))
{
return s.ToString();
nkolev92 marked this conversation as resolved.
Show resolved Hide resolved
}

return null;
}

private static bool TargetFrameworkName_CompatibilityTest(object criteria, object available)
Expand Down
Loading