Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

use buffer pool in newDelimitedReader #71

Merged
merged 3 commits into from
Apr 12, 2019
Merged

use buffer pool in newDelimitedReader #71

merged 3 commits into from
Apr 12, 2019

Conversation

vyzo
Copy link
Contributor

@vyzo vyzo commented Apr 11, 2019

instead of allocating the buffer flat, given how short-lived it is.

instead of allocating the buffer flat
@vyzo vyzo requested a review from Stebalien April 11, 2019 08:49
@ghost ghost assigned vyzo Apr 11, 2019
@ghost ghost added the status/in-progress In progress label Apr 11, 2019
@vyzo
Copy link
Contributor Author

vyzo commented Apr 11, 2019

Actually there is no useful place to put the buffer back in the pool.

@vyzo vyzo closed this Apr 11, 2019
@ghost ghost removed the status/in-progress In progress label Apr 11, 2019
@vyzo vyzo reopened this Apr 11, 2019
@ghost ghost added the status/in-progress In progress label Apr 11, 2019
@vyzo
Copy link
Contributor Author

vyzo commented Apr 11, 2019

I added a Close method in the reader to release the buffer.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Note: We could also just use a one-off pool for delimitedReaders to avoid allocating them at all.

@vyzo
Copy link
Contributor Author

vyzo commented Apr 11, 2019

The global pool probably works just as well for this, and they can be reused by the hop stream buffers.

@vyzo
Copy link
Contributor Author

vyzo commented Apr 11, 2019

Oh you mean a pool for the actual readers. Yeah, maybe so.

@Stebalien
Copy link
Member

Yep.

@vyzo
Copy link
Contributor Author

vyzo commented Apr 12, 2019

I will merge as is, as using a pool for the delimitedReaders themselves adds quite a bit of ugly complexity (we need to track the buffers in two steps).

@vyzo vyzo merged commit d07cd5f into master Apr 12, 2019
@ghost ghost removed the status/in-progress In progress label Apr 12, 2019
@vyzo vyzo deleted the fix/use-buffer-pool branch April 12, 2019 07:49
@Stebalien
Copy link
Member

Well, we could just leave the buffers inside the delimitedReader pool. But either way.

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.

2 participants