Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Commit 308dd10

Browse files
rynowakrynowak
committed
Reduce allocations on Conneg hotpath
1 parent f050e09 commit 308dd10

File tree

7 files changed

+82
-62
lines changed

7 files changed

+82
-62
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ private static readonly HttpHeaderParser<CacheControlHeaderValue> Parser
4949
private ICollection<string> _privateHeaders;
5050
private bool _mustRevalidate;
5151
private bool _proxyRevalidate;
52-
private ICollection<NameValueHeaderValue> _extensions;
52+
private IList<NameValueHeaderValue> _extensions;
5353

5454
public CacheControlHeaderValue()
5555
{
@@ -158,7 +158,7 @@ public bool ProxyRevalidate
158158
set { _proxyRevalidate = value; }
159159
}
160160

161-
public ICollection<NameValueHeaderValue> Extensions
161+
public IList<NameValueHeaderValue> Extensions
162162
{
163163
get
164164
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ private static readonly HttpHeaderParser<ContentDispositionHeaderValue> Parser
2424
= new GenericHeaderParser<ContentDispositionHeaderValue>(false, GetDispositionTypeLength);
2525

2626
// Use list instead of dictionary since we may have multiple parameters with the same name.
27-
private ICollection<NameValueHeaderValue> _parameters;
27+
private ObjectCollection<NameValueHeaderValue> _parameters;
2828
private string _dispositionType;
2929

3030
private ContentDispositionHeaderValue()
@@ -48,7 +48,7 @@ public string DispositionType
4848
}
4949
}
5050

51-
public ICollection<NameValueHeaderValue> Parameters
51+
public IList<NameValueHeaderValue> Parameters
5252
{
5353
get
5454
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public static class HeaderUtilities
1313
private const string QualityName = "q";
1414
internal const string BytesUnit = "bytes";
1515

16-
internal static void SetQuality(ICollection<NameValueHeaderValue> parameters, double? value)
16+
internal static void SetQuality(IList<NameValueHeaderValue> parameters, double? value)
1717
{
1818
Contract.Requires(parameters != null);
1919

@@ -50,7 +50,7 @@ internal static void SetQuality(ICollection<NameValueHeaderValue> parameters, do
5050
}
5151
}
5252

53-
internal static double? GetQuality(ICollection<NameValueHeaderValue> parameters)
53+
internal static double? GetQuality(IList<NameValueHeaderValue> parameters)
5454
{
5555
Contract.Requires(parameters != null);
5656

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

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public string Charset
5959
// Remove charset parameter
6060
if (charsetParameter != null)
6161
{
62-
_parameters.Remove(charsetParameter);
62+
Parameters.Remove(charsetParameter);
6363
}
6464
}
6565
else
@@ -123,7 +123,7 @@ public string Boundary
123123
// Remove charset parameter
124124
if (boundaryParameter != null)
125125
{
126-
_parameters.Remove(boundaryParameter);
126+
Parameters.Remove(boundaryParameter);
127127
}
128128
}
129129
else
@@ -140,7 +140,7 @@ public string Boundary
140140
}
141141
}
142142

143-
public ICollection<NameValueHeaderValue> Parameters
143+
public IList<NameValueHeaderValue> Parameters
144144
{
145145
get
146146
{
@@ -161,8 +161,12 @@ public ICollection<NameValueHeaderValue> Parameters
161161

162162
public double? Quality
163163
{
164-
get { return HeaderUtilities.GetQuality(Parameters); }
165-
set { HeaderUtilities.SetQuality(Parameters, value); }
164+
get { return HeaderUtilities.GetQuality(_parameters); }
165+
set
166+
{
167+
HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
168+
HeaderUtilities.SetQuality(Parameters, value);
169+
}
166170
}
167171

168172
public string MediaType
@@ -210,7 +214,7 @@ public bool MatchesAllSubTypes
210214
{
211215
get
212216
{
213-
return SubType.Equals("*", StringComparison.Ordinal);
217+
return string.Compare(_mediaType, _mediaType.IndexOf('/') + 1, "*", 0, 1, StringComparison.Ordinal) == 0;
214218
}
215219
}
216220

@@ -241,15 +245,31 @@ public bool IsSubsetOf(MediaTypeHeaderValue otherMediaType)
241245
return false;
242246
}
243247

248+
// PERF: Avoid doing anything here that allocates a substring, this is a very hot path
249+
// for content-negotiation.
250+
var indexOfSlash = _mediaType.IndexOf('/');
251+
244252
// "text/plain" is a subset of "text/plain", "text/*" and "*/*". "*/*" is a subset only of "*/*".
245-
if (!Type.Equals(otherMediaType.Type, StringComparison.OrdinalIgnoreCase))
253+
if (string.Compare(
254+
strA: _mediaType,
255+
indexA: 0,
256+
strB: otherMediaType._mediaType,
257+
indexB: 0,
258+
length: indexOfSlash,
259+
comparisonType: StringComparison.OrdinalIgnoreCase) != 0)
246260
{
247261
if (!otherMediaType.MatchesAllTypes)
248262
{
249263
return false;
250264
}
251265
}
252-
else if (!SubType.Equals(otherMediaType.SubType, StringComparison.OrdinalIgnoreCase))
266+
else if (string.Compare(
267+
strA: MediaType,
268+
indexA: indexOfSlash + 1,
269+
strB: otherMediaType._mediaType,
270+
indexB: indexOfSlash + 1, // We know the Type is equal, so the index of '/' is the same in both strings.
271+
length: _mediaType.Length - indexOfSlash,
272+
comparisonType: StringComparison.OrdinalIgnoreCase) != 0)
253273
{
254274
if (!otherMediaType.MatchesAllSubTypes)
255275
{
@@ -457,17 +477,17 @@ private static int GetMediaTypeExpressionLength(string input, int startIndex, ou
457477

458478
// If there is no whitespace between <type> and <subtype> in <type>/<subtype> get the media type using
459479
// one Substring call. Otherwise get substrings for <type> and <subtype> and combine them.
460-
var mediatTypeLength = current + subtypeLength - startIndex;
461-
if (typeLength + subtypeLength + 1 == mediatTypeLength)
480+
var mediaTypeLength = current + subtypeLength - startIndex;
481+
if (typeLength + subtypeLength + 1 == mediaTypeLength)
462482
{
463-
mediaType = input.Substring(startIndex, mediatTypeLength);
483+
mediaType = input.Substring(startIndex, mediaTypeLength);
464484
}
465485
else
466486
{
467487
mediaType = input.Substring(startIndex, typeLength) + "/" + input.Substring(current, subtypeLength);
468488
}
469489

470-
return mediatTypeLength;
490+
return mediaTypeLength;
471491
}
472492

473493
private static void CheckMediaTypeFormat(string mediaType, string parameterName)

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public override string ToString()
173173
}
174174

175175
internal static void ToString(
176-
ICollection<NameValueHeaderValue> values,
176+
IList<NameValueHeaderValue> values,
177177
char separator,
178178
bool leadingSeparator,
179179
StringBuilder destination)
@@ -185,18 +185,18 @@ internal static void ToString(
185185
return;
186186
}
187187

188-
foreach (var value in values)
188+
for (var i = 0; i < values.Count; i++)
189189
{
190190
if (leadingSeparator || (destination.Length > 0))
191191
{
192192
destination.Append(separator);
193193
destination.Append(' ');
194194
}
195-
destination.Append(value.ToString());
195+
destination.Append(values[i].ToString());
196196
}
197197
}
198198

199-
internal static string ToString(ICollection<NameValueHeaderValue> values, char separator, bool leadingSeparator)
199+
internal static string ToString(IList<NameValueHeaderValue> values, char separator, bool leadingSeparator)
200200
{
201201
if ((values == null) || (values.Count == 0))
202202
{
@@ -210,17 +210,17 @@ internal static string ToString(ICollection<NameValueHeaderValue> values, char s
210210
return sb.ToString();
211211
}
212212

213-
internal static int GetHashCode(ICollection<NameValueHeaderValue> values)
213+
internal static int GetHashCode(IList<NameValueHeaderValue> values)
214214
{
215215
if ((values == null) || (values.Count == 0))
216216
{
217217
return 0;
218218
}
219219

220220
var result = 0;
221-
foreach (var value in values)
221+
for (var i = 0; i < values.Count; i++)
222222
{
223-
result = result ^ value.GetHashCode();
223+
result = result ^ values[i].GetHashCode();
224224
}
225225
return result;
226226
}
@@ -282,7 +282,7 @@ internal static int GetNameValueListLength(
282282
string input,
283283
int startIndex,
284284
char delimiter,
285-
ICollection<NameValueHeaderValue> nameValueCollection)
285+
IList<NameValueHeaderValue> nameValueCollection)
286286
{
287287
Contract.Requires(nameValueCollection != null);
288288
Contract.Requires(startIndex >= 0);
@@ -320,7 +320,7 @@ internal static int GetNameValueListLength(
320320
}
321321
}
322322

323-
public static NameValueHeaderValue Find(ICollection<NameValueHeaderValue> values, string name)
323+
public static NameValueHeaderValue Find(IList<NameValueHeaderValue> values, string name)
324324
{
325325
Contract.Requires((name != null) && (name.Length > 0));
326326

@@ -329,8 +329,9 @@ public static NameValueHeaderValue Find(ICollection<NameValueHeaderValue> values
329329
return null;
330330
}
331331

332-
foreach (var value in values)
332+
for (var i = 0; i < values.Count; i++)
333333
{
334+
var value = values[i];
334335
if (string.Compare(value.Name, name, StringComparison.OrdinalIgnoreCase) == 0)
335336
{
336337
return value;

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

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Collections;
65
using System.Collections.Generic;
76
using System.Collections.ObjectModel;
87

@@ -12,74 +11,74 @@ namespace Microsoft.Net.Http.Headers
1211
// type to throw if 'null' gets added. Collection<T> internally uses List<T> which comes at some cost. In addition
1312
// Collection<T>.Add() calls List<T>.InsertItem() which is an O(n) operation (compared to O(1) for List<T>.Add()).
1413
// This type is only used for very small collections (1-2 items) to keep the impact of using Collection<T> small.
15-
internal class ObjectCollection<T> : ICollection<T> where T : class
14+
internal class ObjectCollection<T> : Collection<T> where T : class
1615
{
1716
internal static readonly Action<T> DefaultValidator = CheckNotNull;
1817
internal static readonly ObjectCollection<T> EmptyReadOnlyCollection
1918
= new ObjectCollection<T>(DefaultValidator, isReadOnly: true);
2019

21-
private readonly Collection<T> _collection = new Collection<T>();
2220
private readonly Action<T> _validator;
23-
private readonly bool _isReadOnly;
21+
22+
// We need to create a 'read-only' inner list for Collection<T> to do the right
23+
// thing.
24+
private static IList<T> CreateInnerList(bool isReadOnly, IEnumerable <T> other = null)
25+
{
26+
var list = other == null ? new List<T>() : new List<T>(other);
27+
if (isReadOnly)
28+
{
29+
return new ReadOnlyCollection<T>(list);
30+
}
31+
else
32+
{
33+
return list;
34+
}
35+
}
2436

2537
public ObjectCollection()
2638
: this(DefaultValidator)
2739
{
2840
}
2941

3042
public ObjectCollection(Action<T> validator, bool isReadOnly = false)
43+
: base(CreateInnerList(isReadOnly))
3144
{
3245
_validator = validator;
33-
_isReadOnly = isReadOnly;
3446
}
3547

3648
public ObjectCollection(IEnumerable<T> other, bool isReadOnly = false)
49+
: base(CreateInnerList(isReadOnly, other))
3750
{
3851
_validator = DefaultValidator;
39-
foreach (T item in other)
52+
foreach (T item in Items)
4053
{
41-
Add(item);
54+
_validator(item);
4255
}
43-
_isReadOnly = isReadOnly;
4456
}
4557

46-
public int Count
47-
{
48-
get { return _collection.Count; }
49-
}
58+
public bool IsReadOnly => ((ICollection<T>)this).IsReadOnly;
5059

51-
public bool IsReadOnly
60+
protected override void ClearItems()
5261
{
53-
get { return _isReadOnly; }
62+
base.ClearItems();
5463
}
5564

56-
public void Add(T item)
65+
protected override void InsertItem(int index, T item)
5766
{
58-
HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
5967
_validator(item);
60-
_collection.Add(item);
68+
base.InsertItem(index, item);
6169
}
6270

63-
public bool Remove(T item)
71+
protected override void RemoveItem(int index)
6472
{
65-
HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
66-
return _collection.Remove(item);
73+
base.RemoveItem(index);
6774
}
6875

69-
public void Clear()
76+
protected override void SetItem(int index, T item)
7077
{
71-
HeaderUtilities.ThrowIfReadOnly(IsReadOnly);
72-
_collection.Clear();
78+
_validator(item);
79+
base.SetItem(index, item);
7380
}
7481

75-
public bool Contains(T item) => _collection.Contains(item);
76-
77-
public void CopyTo(T[] array, int arrayIndex) => _collection.CopyTo(array, arrayIndex);
78-
79-
public IEnumerator<T> GetEnumerator() => _collection.GetEnumerator();
80-
81-
IEnumerator IEnumerable.GetEnumerator() => _collection.GetEnumerator();
82-
8382
private static void CheckNotNull(T item)
8483
{
8584
// null values cannot be added to the collection.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ public void CopyAsReadOnly_WithParameters_CopiedAndReadOnly()
122122
Assert.False(mediaType0.Parameters.IsReadOnly);
123123
Assert.True(mediaType1.Parameters.IsReadOnly);
124124
Assert.Equal(mediaType0.Parameters.Count, mediaType1.Parameters.Count);
125-
Assert.Throws<InvalidOperationException>(() => mediaType1.Parameters.Add(new NameValueHeaderValue("name")));
126-
Assert.Throws<InvalidOperationException>(() => mediaType1.Parameters.Remove(new NameValueHeaderValue("name")));
127-
Assert.Throws<InvalidOperationException>(() => mediaType1.Parameters.Clear());
125+
Assert.Throws<NotSupportedException>(() => mediaType1.Parameters.Add(new NameValueHeaderValue("name")));
126+
Assert.Throws<NotSupportedException>(() => mediaType1.Parameters.Remove(new NameValueHeaderValue("name")));
127+
Assert.Throws<NotSupportedException>(() => mediaType1.Parameters.Clear());
128128

129129
var pair0 = mediaType0.Parameters.First();
130130
var pair1 = mediaType1.Parameters.First();

0 commit comments

Comments
 (0)