Skip to content

Commit

Permalink
Reduce allocations in TokenSegment.TryMatch - Use ReadOnlyMemory<char…
Browse files Browse the repository at this point in the history 
…> over string (#5933)
  • Loading branch information
@nkolev92
nkolev92 committed Jul 31, 2024
1 parent 51e6ffe commit 63b869a
Show file tree
Hide file tree
Showing 19 changed files with 152 additions and 229 deletions.
89 changes: 0 additions & 89 deletions src/NuGet.Core/NuGet.Packaging/ArrayPool.cs
View file

This file was deleted.

70 changes: 17 additions & 53 deletions src/NuGet.Core/NuGet.Packaging/ContentModel/ContentPropertyDefinition.cs
View file
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(
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
4 changes: 2 additions & 2 deletions src/NuGet.Core/NuGet.Packaging/ContentModel/Infrastructure/Parser.cs
View file
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
39 changes: 21 additions & 18 deletions src/NuGet.Core/NuGet.Packaging/ContentModel/ManagedCodeConventions.cs
View file
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();
}

return null;
}

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

0 comments on commit 63b869a

Please sign in to comment.