Reduce allocations on Conneg hotpath

src/Microsoft.Net.Http.Headers/CacheControlHeaderValue.cs

@@ -49,7 +49,7 @@ namespace Microsoft.Net.Http.Headers
         private ICollection<string> _privateHeaders;
         private bool _mustRevalidate;
         private bool _proxyRevalidate;
-        private ICollection<NameValueHeaderValue> _extensions;
+        private IList<NameValueHeaderValue> _extensions;
 
         public CacheControlHeaderValue()
         {
@@ -158,7 +158,7 @@ namespace Microsoft.Net.Http.Headers
             set { _proxyRevalidate = value; }
         }
 
-        public ICollection<NameValueHeaderValue> Extensions
+        public IList<NameValueHeaderValue> Extensions
         {
             get
             {

src/Microsoft.Net.Http.Headers/ContentDispositionHeaderValue.cs

@@ -24,7 +24,7 @@ namespace Microsoft.Net.Http.Headers
             = new GenericHeaderParser<ContentDispositionHeaderValue>(false, GetDispositionTypeLength);
 
         // Use list instead of dictionary since we may have multiple parameters with the same name.
-        private ICollection<NameValueHeaderValue> _parameters;
+        private ObjectCollection<NameValueHeaderValue> _parameters;
         private string _dispositionType;
 
         private ContentDispositionHeaderValue()
@@ -48,7 +48,7 @@ namespace Microsoft.Net.Http.Headers
             }
         }
 
-        public ICollection<NameValueHeaderValue> Parameters
+        public IList<NameValueHeaderValue> Parameters
         {
             get
             {

src/Microsoft.Net.Http.Headers/HeaderUtilities.cs

@@ -13,7 +13,7 @@ namespace Microsoft.Net.Http.Headers
         private const string QualityName = "q";
         internal const string BytesUnit = "bytes";
 
-        internal static void SetQuality(ICollection<NameValueHeaderValue> parameters, double? value)
+        internal static void SetQuality(IList<NameValueHeaderValue> parameters, double? value)
         {
             Contract.Requires(parameters != null);
 
@@ -50,7 +50,7 @@ namespace Microsoft.Net.Http.Headers
             }
         }
 
-        internal static double? GetQuality(ICollection<NameValueHeaderValue> parameters)
+        internal static double? GetQuality(IList<NameValueHeaderValue> parameters)
         {
             Contract.Requires(parameters != null);
 

src/Microsoft.Net.Http.Headers/MediaTypeHeaderValue.cs

@@ -59,7 +59,7 @@ namespace Microsoft.Net.Http.Headers
                     // Remove charset parameter
                     if (charsetParameter != null)
                     {
-                        _parameters.Remove(charsetParameter);
+                        Parameters.Remove(charsetParameter);
                     }
                 }
                 else
@@ -123,7 +123,7 @@ namespace Microsoft.Net.Http.Headers
                     // Remove charset parameter
                     if (boundaryParameter != null)
                     {
-                        _parameters.Remove(boundaryParameter);
+                        Parameters.Remove(boundaryParameter);
                     }
                 }
                 else
@@ -140,7 +140,7 @@ namespace Microsoft.Net.Http.Headers
             }
         }
 
-        public ICollection<NameValueHeaderValue> Parameters
+        public IList<NameValueHeaderValue> Parameters
         {
             get
             {
@@ -161,8 +161,12 @@ namespace Microsoft.Net.Http.Headers
 
         public double? Quality
         {
-            get { return HeaderUtilities.GetQuality(Parameters); }
+            get { return HeaderUtilities.GetQuality(_parameters); }
-            set { HeaderUtilities.SetQuality(Parameters, value); }
+            set
+            {
+                HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
+                HeaderUtilities.SetQuality(Parameters, value);
+            }
         }
 
         public string MediaType
@@ -210,7 +214,7 @@ namespace Microsoft.Net.Http.Headers
         {
             get
             {
-                return SubType.Equals("*", StringComparison.Ordinal);
+                return string.Compare(_mediaType, _mediaType.IndexOf('/') + 1, "*", 0, 1, StringComparison.Ordinal) == 0;
             }
         }
 
@@ -241,15 +245,31 @@ namespace Microsoft.Net.Http.Headers
                 return false;
             }
 
+            // PERF: Avoid doing anything here that allocates a substring, this is a very hot path
+            // for content-negotiation.
+            var indexOfSlash = _mediaType.IndexOf('/');
+
             // "text/plain" is a subset of "text/plain", "text/*" and "*/*". "*/*" is a subset only of "*/*".
-            if (!Type.Equals(otherMediaType.Type, StringComparison.OrdinalIgnoreCase))
+            if (string.Compare(
+                strA: _mediaType,
+                indexA: 0,
+                strB: otherMediaType._mediaType,
+                indexB: 0,
+                length: indexOfSlash,
+                comparisonType: StringComparison.OrdinalIgnoreCase) != 0)
             {
                 if (!otherMediaType.MatchesAllTypes)
                 {
                     return false;
                 }
             }
-            else if (!SubType.Equals(otherMediaType.SubType, StringComparison.OrdinalIgnoreCase))
+            else if (string.Compare(
+                strA: MediaType,
+                indexA: indexOfSlash + 1,
+                strB: otherMediaType._mediaType,
+                indexB: indexOfSlash + 1, // We know the Type is equal, so the index of '/' is the same in both strings.
+                length: _mediaType.Length - indexOfSlash,
+                comparisonType: StringComparison.OrdinalIgnoreCase) != 0)
             {
                 if (!otherMediaType.MatchesAllSubTypes)
                 {
@@ -457,17 +477,17 @@ namespace Microsoft.Net.Http.Headers
 
             // If there is no whitespace between <type> and <subtype> in <type>/<subtype> get the media type using
             // one Substring call. Otherwise get substrings for <type> and <subtype> and combine them.
-            var mediatTypeLength = current + subtypeLength - startIndex;
+            var mediaTypeLength = current + subtypeLength - startIndex;
-            if (typeLength + subtypeLength + 1 == mediatTypeLength)
+            if (typeLength + subtypeLength + 1 == mediaTypeLength)
             {
-                mediaType = input.Substring(startIndex, mediatTypeLength);
+                mediaType = input.Substring(startIndex, mediaTypeLength);
             }
             else
             {
                 mediaType = input.Substring(startIndex, typeLength) + "/" + input.Substring(current, subtypeLength);
             }
 
-            return mediatTypeLength;
+            return mediaTypeLength;
         }
 
         private static void CheckMediaTypeFormat(string mediaType, string parameterName)

src/Microsoft.Net.Http.Headers/NameValueHeaderValue.cs

@@ -173,7 +173,7 @@ namespace Microsoft.Net.Http.Headers
         }
 
         internal static void ToString(
-            ICollection<NameValueHeaderValue> values,
+            IList<NameValueHeaderValue> values,
             char separator,
             bool leadingSeparator,
             StringBuilder destination)
@@ -185,18 +185,18 @@ namespace Microsoft.Net.Http.Headers
                 return;
             }
 
-            foreach (var value in values)
+            for (var i = 0; i < values.Count; i++)
             {
                 if (leadingSeparator || (destination.Length > 0))
                 {
                     destination.Append(separator);
                     destination.Append(' ');
                 }
-                destination.Append(value.ToString());
+                destination.Append(values[i].ToString());
             }
         }
 
-        internal static string ToString(ICollection<NameValueHeaderValue> values, char separator, bool leadingSeparator)
+        internal static string ToString(IList<NameValueHeaderValue> values, char separator, bool leadingSeparator)
         {
             if ((values == null) || (values.Count == 0))
             {
@@ -210,7 +210,7 @@ namespace Microsoft.Net.Http.Headers
             return sb.ToString();
         }
 
-        internal static int GetHashCode(ICollection<NameValueHeaderValue> values)
+        internal static int GetHashCode(IList<NameValueHeaderValue> values)
         {
             if ((values == null) || (values.Count == 0))
             {
@@ -218,9 +218,9 @@ namespace Microsoft.Net.Http.Headers
             }
 
             var result = 0;
-            foreach (var value in values)
+            for (var i = 0; i < values.Count; i++)
             {
-                result = result ^ value.GetHashCode();
+                result = result ^ values[i].GetHashCode();
             }
             return result;
         }
@@ -282,7 +282,7 @@ namespace Microsoft.Net.Http.Headers
             string input,
             int startIndex,
             char delimiter,
-            ICollection<NameValueHeaderValue> nameValueCollection)
+            IList<NameValueHeaderValue> nameValueCollection)
         {
             Contract.Requires(nameValueCollection != null);
             Contract.Requires(startIndex >= 0);
@@ -320,7 +320,7 @@ namespace Microsoft.Net.Http.Headers
             }
         }
 
-        public static NameValueHeaderValue Find(ICollection<NameValueHeaderValue> values, string name)
+        public static NameValueHeaderValue Find(IList<NameValueHeaderValue> values, string name)
         {
             Contract.Requires((name != null) && (name.Length > 0));
 
@@ -329,8 +329,9 @@ namespace Microsoft.Net.Http.Headers
                 return null;
             }
 
-            foreach (var value in values)
+            for (var i = 0; i < values.Count; i++)
             {
+                var value = values[i];
                 if (string.Compare(value.Name, name, StringComparison.OrdinalIgnoreCase) == 0)
                 {
                     return value;

src/Microsoft.Net.Http.Headers/ObjectCollection.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 
@@ -12,15 +11,28 @@ namespace Microsoft.Net.Http.Headers
     // type to throw if 'null' gets added. Collection<T> internally uses List<T> which comes at some cost. In addition
     // Collection<T>.Add() calls List<T>.InsertItem() which is an O(n) operation (compared to O(1) for List<T>.Add()).
     // This type is only used for very small collections (1-2 items) to keep the impact of using Collection<T> small.
-    internal class ObjectCollection<T> : ICollection<T> where T : class
+    internal class ObjectCollection<T> : Collection<T> where T : class
     {
         internal static readonly Action<T> DefaultValidator = CheckNotNull;
         internal static readonly ObjectCollection<T> EmptyReadOnlyCollection
             = new ObjectCollection<T>(DefaultValidator, isReadOnly: true);
 
-        private readonly Collection<T> _collection = new Collection<T>();
         private readonly Action<T> _validator;
-        private readonly bool _isReadOnly;
+
+        // We need to create a 'read-only' inner list for Collection<T> to do the right
+        // thing.
+        private static IList<T> CreateInnerList(bool isReadOnly, IEnumerable <T> other = null)
+        {
+            var list = other == null ? new List<T>() : new List<T>(other);
+            if (isReadOnly)
+            {
+                return new ReadOnlyCollection<T>(list);
+            }
+            else
+            {
+                return list;
+            }
+        }
 
         public ObjectCollection()
             : this(DefaultValidator)
@@ -28,58 +40,45 @@ namespace Microsoft.Net.Http.Headers
         }
 
         public ObjectCollection(Action<T> validator, bool isReadOnly = false)
+            : base(CreateInnerList(isReadOnly))
         {
             _validator = validator;
-            _isReadOnly = isReadOnly;
         }
 
         public ObjectCollection(IEnumerable<T> other, bool isReadOnly = false)
+            : base(CreateInnerList(isReadOnly, other))
         {
             _validator = DefaultValidator;
-            foreach (T item in other)
+            foreach (T item in Items)
             {
-                Add(item);
+                _validator(item);
             }
-            _isReadOnly = isReadOnly;
         }
 
-        public int Count
+        public bool IsReadOnly => ((ICollection<T>)this).IsReadOnly;
+
+        protected override void ClearItems()
         {
-            get { return _collection.Count; }
+            base.ClearItems();
         }
 
-        public bool IsReadOnly
+        protected override void InsertItem(int index, T item)
         {
-            get { return _isReadOnly; }
-        }
-
-        public void Add(T item)
-        {
-            HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
             _validator(item);
-            _collection.Add(item);
+            base.InsertItem(index, item);
         }
 
-        public bool Remove(T item)
+        protected override void RemoveItem(int index)
         {
-            HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
+            base.RemoveItem(index);
-            return _collection.Remove(item);
         }
 
-        public void Clear()
+        protected override void SetItem(int index, T item)
         {
-            HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
+            _validator(item);
-            _collection.Clear();
+            base.SetItem(index, item);
         }
 
-        public bool Contains(T item) => _collection.Contains(item);
-
-        public void CopyTo(T[] array, int arrayIndex) => _collection.CopyTo(array, arrayIndex);
-
-        public IEnumerator<T> GetEnumerator() => _collection.GetEnumerator();
-
-        IEnumerator IEnumerable.GetEnumerator() => _collection.GetEnumerator();
-
         private static void CheckNotNull(T item)
         {
             // null values cannot be added to the collection.

test/Microsoft.Net.Http.Headers.Tests/MediaTypeHeaderValueTest.cs

@@ -122,9 +122,9 @@ namespace Microsoft.Net.Http.Headers
             Assert.False(mediaType0.Parameters.IsReadOnly);
             Assert.True(mediaType1.Parameters.IsReadOnly);
             Assert.Equal(mediaType0.Parameters.Count, mediaType1.Parameters.Count);
-            Assert.Throws<InvalidOperationException>(() => mediaType1.Parameters.Add(new NameValueHeaderValue("name")));
+            Assert.Throws<NotSupportedException>(() => mediaType1.Parameters.Add(new NameValueHeaderValue("name")));
-            Assert.Throws<InvalidOperationException>(() => mediaType1.Parameters.Remove(new NameValueHeaderValue("name")));
+            Assert.Throws<NotSupportedException>(() => mediaType1.Parameters.Remove(new NameValueHeaderValue("name")));
-            Assert.Throws<InvalidOperationException>(() => mediaType1.Parameters.Clear());
+            Assert.Throws<NotSupportedException>(() => mediaType1.Parameters.Clear());
 
             var pair0 = mediaType0.Parameters.First();
             var pair1 = mediaType1.Parameters.First();