Reimplement System.Net.Http's ObjectCollection<T> to reduce allocation / interface dispatch#34902
Merged
stephentoub merged 1 commit intodotnet:masterfrom Apr 13, 2020
Conversation
…n / interface dispatch The current implementation of `ObjectCollection<T>` wraps a `List<T>` and derives from `Collection<T>`. This means that every `ObjectCollection<T>` allocated also involves an extra `List<T>` object (and its array if items are added to it) as well as multiple levels of indirection on each operation. We can instead just implement `ObjectCollection<T>` directly. Since most uses end up with just a single object contained (e.g. for a `MedaTypeHeaderValue`'s `CharSet`), and since it only ever stores `T`s that are non-null classes, we can use the items field to be either a `T` or a `T[]`, optimizing for the case where a single element is stored. In implementing it directly, we then also avoid the extra `List<T>` object allocation, as well as the interface dispatch that results from going through the `IList<T>` interface.
|
Tagging @dotnet/ncl as an area owner. If you would like to be tagged for a label, please notify danmosemsft. |
scalablecory
approved these changes
Apr 13, 2020
Contributor
scalablecory
left a comment
There was a problem hiding this comment.
Code looks good; have you also benchmarked the >1 item case?
Member
Author
Yup: [Params(1, 2, 4, 8)]
public int Parameters { get; set; }
[Benchmark]
public void Create()
{
var m = new MediaTypeHeaderValue("application/json");
for (int i = 0; i < Parameters; i++)
{
m.Parameters.Add(new NameValueHeaderValue(i.ToString(), "value"));
}
}
|
davidsh
approved these changes
Apr 13, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current implementation of
ObjectCollection<T>wraps aList<T>and derives fromCollection<T>. This means that everyObjectCollection<T>allocated also involves an extraList<T>object (and its array if items are added to it) as well as multiple levels of indirection on each operation.We can instead just implement
ObjectCollection<T>directly. Since most uses end up with just a single object contained (e.g. for aMedaTypeHeaderValue'sCharSet), and since it only ever storesTs that are non-null classes, we can use the items field to be either aTor aT[], optimizing for the case where a single element is stored. In implementing it directly, we then also avoid the extraList<T>object allocation, as well as the interface dispatch that results from going through theIList<T>interface.This shows up in simple HTTP requests because many requests end up sending down a Content-Type response header, often including a charset, and if headers are enumerated, that results in a MediaTypeHeaderValue being created.
cc: @scalablecory, @davidsh