Propagate match only in asset selection to reduce allocations when gr…

…ouping (#5944)

src/NuGet.Core/NuGet.Packaging/ContentModel/ContentPropertyDefinition.cs

@@ -17,21 +17,21 @@ public class ContentPropertyDefinition
 
         internal ContentPropertyDefinition(
             string name,
-            Func<ReadOnlyMemory<char>, PatternTable, object> parser)
+            Func<ReadOnlyMemory<char>, PatternTable, bool, object> parser)
             : this(name, parser, null, null, null, false)
         {
         }
 
         internal ContentPropertyDefinition(
             string name,
-            Func<ReadOnlyMemory<char>, PatternTable, object> parser,
+            Func<ReadOnlyMemory<char>, PatternTable, bool, object> parser,
             Func<object, object, bool> compatibilityTest)
             : this(name, parser, compatibilityTest, null, null, false)
         {
         }
 
         internal ContentPropertyDefinition(string name,
-            Func<ReadOnlyMemory<char>, PatternTable, object> parser,
+            Func<ReadOnlyMemory<char>, PatternTable, bool, object> parser,
             Func<object, object, bool> compatibilityTest,
             Func<object, object, object, int> compareTest)
             : this(name, parser, compatibilityTest, compareTest, null, false)
@@ -40,15 +40,15 @@ internal ContentPropertyDefinition(string name,
 
         internal ContentPropertyDefinition(
             string name,
-            Func<ReadOnlyMemory<char>, PatternTable, object> parser,
+            Func<ReadOnlyMemory<char>, PatternTable, bool, object> parser,
             IEnumerable<string> fileExtensions)
             : this(name, parser, null, null, fileExtensions, false)
         {
         }
 
         internal ContentPropertyDefinition(
             string name,
-            Func<ReadOnlyMemory<char>, PatternTable, object> parser,
+            Func<ReadOnlyMemory<char>, PatternTable, bool, object> parser,
             Func<object, object, bool> compatibilityTest,
             Func<object, object, object, int> compareTest,
             IEnumerable<string> fileExtensions,
@@ -68,9 +68,27 @@ internal ContentPropertyDefinition(
 
         public bool FileExtensionAllowSubFolders { get; }
 
-        internal Func<ReadOnlyMemory<char>, PatternTable, object> Parser { get; }
+        /// <summary>
+        /// Parse a ReadOnlyMemory char if it's off the form of this definition.
+        /// A null return value means the ReadOnlyMemory char does not match this definition.
+        /// If the bool is true, the return object will be non-null, and match what the ReadOnlyMemory char represents.
+        /// If the bool is false, the return object will be non-null if the ReadOnlyMemory char represents a valid value for this definition. This is a performance optimization.
+        /// </summary>
+        internal Func<ReadOnlyMemory<char>, PatternTable, bool, object> Parser { get; }
 
-        internal virtual bool TryLookup(ReadOnlyMemory<char> name, PatternTable table, out object value)
+        /// <summary>
+        /// Looks up a definition for the given substring.
+        /// Example, say this definition is for an assembly. 
+        /// If the name is "assembly.dll", this method would return true and the value would be the assembly name.
+        /// If the name is "assembly.xml" this method would return flase and the value would be the null.
+        /// If this is a match only lookup the value will be null, but the return bool will be true. This is a performance optimization since the value is unused.
+        /// </summary>
+        /// <param name="name">The name to lookup.</param>
+        /// <param name="table">A replacement table. If name matches a value in the replacement table, it'll be returned instead. </param>
+        /// <param name="matchOnly">Whether this is a grouping match, or we actually want to actualize the value of name as a string.</param>
+        /// <param name="value">The out param. If matchonly, it will always be null. Otherwise, set to actualized value of name if the return is true, set to null if false.</param>
+        /// <returns>True if the name matches the definition. False otherwise.</returns>
+        internal virtual bool TryLookup(ReadOnlyMemory<char> name, PatternTable table, bool matchOnly, out object value)
         {
             if (name.IsEmpty)
             {
@@ -86,7 +104,14 @@ internal virtual bool TryLookup(ReadOnlyMemory<char> name, PatternTable table, o
                     {
                         if (name.Span.EndsWith(fileExtension.AsSpan(), StringComparison.OrdinalIgnoreCase))
                         {
-                            value = name.ToString();
+                            if (!matchOnly)
+                            {
+                                value = name.ToString();
+                            }
+                            else
+                            {
+                                value = null;
+                            }
                             return true;
                         }
                     }
@@ -95,7 +120,7 @@ internal virtual bool TryLookup(ReadOnlyMemory<char> name, PatternTable table, o
 
             if (Parser != null)
             {
-                value = Parser.Invoke(name, table);
+                value = Parser.Invoke(name, table, matchOnly);
                 if (value != null)
                 {
                     return true;

src/NuGet.Core/NuGet.Packaging/ContentModel/Infrastructure/Parser.cs

@@ -151,13 +151,15 @@ private class TokenSegment : Segment
             private readonly char _delimiter;
             private readonly bool _matchOnly;
             private readonly PatternTable _table;
+            private readonly bool _preserveRawValue = false;
 
             public TokenSegment(string token, char delimiter, bool matchOnly, PatternTable table)
             {
                 _token = token;
                 _delimiter = delimiter;
                 _matchOnly = matchOnly;
                 _table = table;
+                _preserveRawValue = StringComparer.Ordinal.Equals(_token, "tfm");
             }
 
             internal override bool TryMatch(
@@ -192,7 +194,7 @@ internal override bool TryMatch(
                     }
                     ReadOnlyMemory<char> substring = path.AsMemory(startIndex, delimiterIndex - startIndex);
                     object value;
-                    if (propertyDefinition.TryLookup(substring, _table, out value))
+                    if (propertyDefinition.TryLookup(substring, _table, _matchOnly, out value))
                     {
                         if (!_matchOnly)
                         {
@@ -204,7 +206,7 @@ internal override bool TryMatch(
                                     Path = path
                                 };
                             }
-                            if (StringComparer.Ordinal.Equals(_token, "tfm"))
+                            if (_preserveRawValue)
                             {
                                 item.Properties.Add("tfm_raw", substring.ToString());
                             }

src/NuGet.Core/NuGet.Packaging/ContentModel/ManagedCodeConventions.cs

@@ -21,7 +21,7 @@ public class ManagedCodeConventions
 
         private static readonly ContentPropertyDefinition AnyProperty = new ContentPropertyDefinition(
             PropertyNames.AnyValue,
-            parser: (o, t) => o.ToString()); // Identity parser, all strings are valid for any
+            parser: IdentityParser); // Identity parser, all strings are valid for any
         private static readonly ContentPropertyDefinition AssemblyProperty = new ContentPropertyDefinition(PropertyNames.ManagedAssembly,
             parser: AllowEmptyFolderParser,
             fileExtensions: new[] { ".dll", ".winmd", ".exe" });
@@ -88,7 +88,7 @@ public ManagedCodeConventions(RuntimeGraph runtimeGraph)
 
             props[PropertyNames.RuntimeIdentifier] = new ContentPropertyDefinition(
                 PropertyNames.RuntimeIdentifier,
-                parser: (o, t) => o.ToString(), // Identity parser, all strings are valid runtime ids
+                parser: IdentityParser, // Identity parser, all strings are valid runtime ids
                 compatibilityTest: RuntimeIdentifier_CompatibilityTest);
 
             props[PropertyNames.TargetFrameworkMoniker] = new ContentPropertyDefinition(
@@ -123,7 +123,11 @@ private bool RuntimeIdentifier_CompatibilityTest(object criteria, object availab
             }
         }
 
-        private static object CodeLanguage_Parser(ReadOnlyMemory<char> name, PatternTable table)
+        /// <summary>
+        /// If matchOnly is true, then an empty string may be returned as a performance optimization.
+        /// If matchOnly is false, the parsed result will be returned.
+        /// </summary>
+        private static object CodeLanguage_Parser(ReadOnlyMemory<char> name, PatternTable table, bool matchOnly)
         {
             if (table != null)
             {
@@ -143,10 +147,20 @@ private static object CodeLanguage_Parser(ReadOnlyMemory<char> name, PatternTabl
                     return null;
                 }
             }
+
+            if (matchOnly)
+            {
+                return string.Empty;
+            }
+
             return name.ToString();
         }
 
-        private static object Locale_Parser(ReadOnlyMemory<char> name, PatternTable table)
+        /// <summary>
+        /// If matchOnly is true, then an empty string may be returned as a performance optimization.
+        /// If matchOnly is false, the parsed result will be returned.
+        /// </summary>
+        private static object Locale_Parser(ReadOnlyMemory<char> name, PatternTable table, bool matchOnly)
         {
             if (table != null)
             {
@@ -159,10 +173,18 @@ private static object Locale_Parser(ReadOnlyMemory<char> name, PatternTable tabl
 
             if (name.Length == 2)
             {
+                if (matchOnly)
+                {
+                    return string.Empty;
+                }
                 return name.ToString();
             }
             else if (name.Length >= 4 && name.Span[2] == '-')
             {
+                if (matchOnly)
+                {
+                    return string.Empty;
+                }
                 return name.ToString();
             }
 
@@ -171,7 +193,8 @@ private static object Locale_Parser(ReadOnlyMemory<char> name, PatternTable tabl
 
         private object TargetFrameworkName_Parser(
             ReadOnlyMemory<char> name,
-            PatternTable table)
+            PatternTable table,
+            bool matchOnly)
         {
             object obj = null;
 
@@ -224,12 +247,35 @@ private static NuGetFramework TargetFrameworkName_ParserCore(string name)
             return new NuGetFramework(name, FrameworkConstants.EmptyVersion);
         }
 
-        private static object AllowEmptyFolderParser(ReadOnlyMemory<char> s, PatternTable table)
+        /// <summary>
+        /// Identity parser, returns the input string as is.
+        /// If matchOnly is true, then an empty string is returned as a performance optimization.
+        /// If matchOnly is false, the string will be actualized.
+        /// </summary>
+        private static object IdentityParser(ReadOnlyMemory<char> s, PatternTable _, bool matchOnly)
+        {
+            if (matchOnly)
+            {
+                return string.Empty;
+            }
+            return s.ToString();
+        }
+
+
+        /// <summary>
+        /// If matchOnly is true, then an empty string is returned as a performance optimization.
+        /// If matchOnly is false, the parsed result will be returned.
+        /// </summary>
+        private static object AllowEmptyFolderParser(ReadOnlyMemory<char> s, PatternTable table, bool matchOnly)
         {
             // Accept "_._" as a pseudo-assembly
             if (MemoryExtensions.Equals(PackagingCoreConstants.EmptyFolder.AsSpan(), s.Span, StringComparison.Ordinal))
             {
-                return s.ToString();
+                if (matchOnly)
+                {
+                    return string.Empty;
+                }
+                return PackagingCoreConstants.EmptyFolder;
             }
 
             return null;

src/NuGet.Core/NuGet.Packaging/ContentModel/SelectionCriteriaBuilder.cs

@@ -56,7 +56,7 @@ internal SelectionCriteriaEntryBuilder(SelectionCriteriaBuilder builder, Selecti
                 else
                 {
                     object valueLookup;
-                    if (propertyDefinition.TryLookup(value.AsMemory(), table: null, value: out valueLookup))
+                    if (propertyDefinition.TryLookup(value.AsMemory(), table: null, matchOnly: false, value: out valueLookup))
                     {
                         Entry.Properties[key] = valueLookup;
                     }