Accurately count only newly examined bytes

[halter73]

• Ensure Kestrel count all bytes read using TryRead
• Ensure Kestrel doesn't double count examined but
  not consumed bytes

See #11942 (comment) for context

[halter73]

TODO: Write regression tests. I just wanted everyone to see the proposed preview8 changes ASAP.

[halter73]

@anurse @davidfowl I think we should take this for preview8.

[davidfowl]

Any potential performance regression here?

[halter73]

Doubt it totalLength = readResult.Buffer.Length; would be the only line that has much of a chance of impacting perf, but that only runs if you don't examine to the end of the buffer which should be exceedingly rare when reading the request body. The super sensitive benchmarks don't have a request body to begin with.

[analogrelay]

I think we should take this for preview8.

Agreed, but just from a bureaucracy perspective, if it doesn't happen I feel confident taking this in preview 9. It seems edge-casey enough that I don't know if I'd block preview8 for this fix, but read-rate accounting is critical to the product so getting it in for preview9 seems fair.

Anyway, that's all moot if we can land it by Monday. Just setting the context and contingency plans.

[jkotalik]

We will do cleanup for preview9. Functionally looks sound to me.

[benaadams]

@halter73 Should this then update _alreadyTimedBytes?

var numFirstSeenBytes = bytesInReadResult - _alreadyTimedBytes;

if (numFirstSeenBytes > 0)
{
    _context.TimeoutControl.BytesRead(numFirstSeenBytes);
    _alreadyTimedBytes = bytesInReadResult ;
}

[benaadams]

Ah, nvm I think its ok

[halter73]

Yeah. We count all the bytes in the ReadResult towards rate calculations, examined or not. Then in Advance, if we don't consume everything, we count all the unconsumed and unexamined bytes towards _alreadyTimedBytes, since both the unconsumed and unexamined bytes will be present in the next read result. What got you looking at this?

[benaadams]

Was getting test failures with #11942 (comment) when I removed all the changes that weren't directly related to the streams; as assumed this superseded them.

Issue wasn't with counting though; needed TryStart/TryStop in Http2MessageBody