Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Fix regressions in FormReader / FileBufferingReadStream. #606

Merged
merged 1 commit into from
Apr 1, 2016

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Apr 1, 2016

#605 @davidfowl @muratg @benaadams @BrennanConroy

The recent perf changes caused two regressions in the FileBufferingReadStream used by form reader. The one reported in #605 is that most of the data buffered in memory gets discarded when transitioning to the file buffer. This happened because Write was accidentally given the wrong length when I cleaned up some of @benaadams's code.

The other regression was that the constructor only rented buffers if the in-memory threshold was less than 1mb, but Read always assumed it was using rented buffers. This would have caused ArgumentNullExceptions trying to use an uninitialized buffer. The fix is to rent a buffer when needed and do a manual CopyTo loop.

Compare with bd60507#diff-6bee66e850efacd8fe96daae334fa1c5R192

@Tratcher Tratcher self-assigned this Apr 1, 2016
@Tratcher Tratcher added this to the 1.0.0-rc2 milestone Apr 1, 2016
_rentedBuffer = null;
if (_rentedBuffer == null)
{
oldBuffer.Position = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It breaks my brain a little bit to look at this code and see 'buffer' used for both Stream and arrays

@BrennanConroy
Copy link
Member

Looks reasonable :shipit:

@Tratcher Tratcher merged commit 8196f2a into dev Apr 1, 2016
@Tratcher Tratcher deleted the tratcher/forms branch April 1, 2016 23:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants