Avoid per-call bool[] allocation in HeaderUtilities.AreEqualCollections#131230
Avoid per-call bool[] allocation in HeaderUtilities.AreEqualCollections#131230mangod9 wants to merge 1 commit into
Conversation
AreEqualCollections allocated a new bool[x.Count] scratch array on every header value equality comparison (CacheControl/MediaType/ContentDisposition/Range/ TransferCoding/NameValueWithParameters). Header parameter counts are almost always 1-2, so use a stackalloc buffer for small counts with an ArrayPool<bool> fallback for larger ones. Also replaces the Array.TrueForAll closure in the debug assert with an allocation-free Span check. Behavior-preserving; System.Net.Http unit tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
There was a problem hiding this comment.
Pull request overview
This PR optimizes HeaderUtilities.AreEqualCollections (used by various header-value Equals implementations in System.Net.Http) by removing a per-call heap allocation in favor of stack allocation for the common small-count case, with an ArrayPool<bool> fallback for larger collections.
Changes:
- Replaces
new bool[x.Count]with astackalloc-backedSpan<bool>(thresholded) andArrayPool<bool>for larger counts, returned viafinally. - Replaces
Array.TrueForAll(..., value => value)(closure) with an allocation-freeSpan<bool>check (Contains(false)) underDebug.Assert.
|
|
||
| // We have two unordered lists. So comparison is an O(n*m) operation which is expensive. Usually | ||
| // headers have 1-2 parameters (if any), so this comparison shouldn't be too expensive. | ||
| bool[] alreadyFound = new bool[x.Count]; |
There was a problem hiding this comment.
@AndyAyersMS this looks like a good candidate for escape analysis so all the unsafe code with stackalloc + arraypool becomes unnecessary
There was a problem hiding this comment.
#127980 will conditionally stack allocate this array. Its threshold is dynamic depending on how much free space exists on the stack. However the fallback is a heap allocation, not an array pool rental.
If we wanted to model this exact pattern we'd have to add quite a bit more logic to #127980 and some hellper-call accessible versions of rent/return.
There was a problem hiding this comment.
I wonder if we can rewrite it to just bool[] alreadyFound = x.Count <= 32 ? new bool[32] : new bool[x.Count]; and it will just work we can assume the number (headers) is rather low.
Removes a per-call scratch-array allocation in
HeaderUtilities.AreEqualCollections, which is invoked from theEqualsoverrides of parameterized header values (CacheControl,MediaType,ContentDisposition,Range,TransferCoding,NameValueWithParameters, ...).Problem
AreEqualCollectionsallocated anew bool[x.Count]on every comparison. As the existing code comment notes, header parameter counts are almost always 1-2, so this is a small heap allocation on a comparison path.Change
Use a
stackalloc bool[32]buffer for the common small case, with anArrayPool<bool>fallback (returned in afinally) for larger counts. Also replaces theArray.TrueForAll(alreadyFound, ...)closure in theDebug.Assertwith an allocation-freeSpancheck. Behavior is unchanged.Validation
System.Net.HttpTFMs, 0 warnings.System.Net.Httpunit tests pass (2677/2677) - the header-valueEqualstests exercise this path.Note
This pull request was generated with the assistance of GitHub Copilot.