Consume body without allocating or reading #335
Conversation
benaadams
force-pushed
the
used-pool-array
branch
10 times, most recently
from
08f774c to
c4d2b74
Compare
benaadams
force-pushed
the
used-pool-array
branch
2 times, most recently
from
34b5fc3 to
79c4a42
Compare
|
This needs to be implemented in a non allocating way. |
|
Not sure I understand, it only allocates if the header name > 4032 bytes so it shouldn't ever allocate? |
|
Skip rather than Read |
benaadams
changed the title
| @@ -566,7 +566,7 @@ public string GetString(MemoryPoolIterator2 end) | |||
| } | |||
| } | |||
|
|
|||
| public ArraySegment<byte> GetArraySegment(MemoryPoolIterator2 end) | |||
| public ArraySegment<byte> GetArraySegment(MemoryPoolIterator2 end, ArraySegment<byte> scratchBuffer) | |||
benaadams
force-pushed
the
used-pool-array
branch
from
01b60d6 to
51056e2
Compare
benaadams
changed the title
benaadams
force-pushed
the
used-pool-array
branch
from
51056e2 to
f089abd
Compare
|
@davidfowl does the last commit sort out your code reuse concerns? |
|
@benaadams Yes, exactly what I had in mind (except the 100 continue changes) |
| var firstLoop = true; | ||
| do | ||
| { | ||
| result = ReadAsyncImplementation(default(ArraySegment<byte>), cancellationToken); |
There was a problem hiding this comment.
Was copying the function above. Following the logic of that function; the task hasn't completed because no data; check whether their is no data because the client is waiting for a 100 continue else the await will block infinitely as it waiting for data that's dependent on a send; if so do the send and return the incomplete task?
Since we are doing the full loop here; if we pass over that condition; we know the task has already completed. If the result is 0 then return as everything is done; otherwise move to next loop, no point in checking twice with the await for a condition we already know. Therefore we only await if the task is not complete.
There was a problem hiding this comment.
Also Interlocked is much more expensive than a local var and only one 100 continue is sent, so only check it once, as we know the results for every loop after that. Though firstLoop is probably the wrong name.
| @@ -85,7 +85,7 @@ public override void Flush() | |||
|
|
|||
| private Task<int> ReadAsync(ArraySegment<byte> buffer) | |||
| { | |||
| return _input.ReadAsync(buffer); | |||
| return _input.ReadAsync(buffer.Array, buffer.Offset, buffer.Count); | |||
There was a problem hiding this comment.
Worth just overloading this on SocketInput? Notice a bunch of other methods here create an array segment to call this method.
At least least, you could clean all of those up to avoid creating the ArraySegment
There was a problem hiding this comment.
Can't create an ArraySegement with a null buffer + specified length or it will throw an exception.
However a consumption limit needs to be passed through to CopyTo else it will consume the whole input rather than just the input related to the current request. Is a bit of abstraction leakiness to make the code reusable; rather than copy paste repeated...
Consume body without allocating or reading
|
🎆 |
Was meant to be removed as part of aspnet#335 /cc @rynowak
Resolves #334