Update shared HPackEncoder #33170
Update shared HPackEncoder #33170
Conversation
JamesNK
force-pushed
the
jamesnk/hpackshared-sync
branch
from
3857c62 to
b201455
Compare
| namespace System.Net.Http.HPack | ||
| { | ||
| internal class HPackEncoder | ||
| internal static class HPackEncoder |
src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackEncoder.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| /// <summary>Encodes the status code of a response to the :status field.</summary> | ||
| public static bool EncodeStatusHeader(int statusCode, Span<byte> destination, out int bytesWritten) |
There was a problem hiding this comment.
I would merge this idea into StatusCodes.ToStatusBytes(). You can have the static arrays there store the pre-encoded version of the full header.
There was a problem hiding this comment.
Good idea but I'll leave that for now. It can be a future optimization.
| // | Value String (Length octets) | | ||
| // +-------------------------------+ | ||
|
|
||
| if ((uint)destination.Length >= 3) |
There was a problem hiding this comment.
What's the purpose of the cast here?
There was a problem hiding this comment.
I assume it is to eliminate bounds checks. I just copied and pasted this from the method overload that takes a span of alreadys instead of one.
scalablecory
left a comment
scalablecory
left a comment
There was a problem hiding this comment.
looks good, only small comments.
|
Merge please. I don't have permission (I should really figure that out...) |
|
/azp run runtime-libraries outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Failure is unrelated. |
ghost
locked as resolved and limited conversation to collaborators
6 participants
Sync changes from dotnet/aspnetcore#19521