Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bufio: no way to give/take underlying buffers #5100

Closed
bradfitz opened this issue Mar 21, 2013 · 10 comments
Closed

bufio: no way to give/take underlying buffers #5100

bradfitz opened this issue Mar 21, 2013 · 10 comments
Labels
FrozenDueToAge GarbageCollector
Milestone
Go1.2

Comments

@bradfitz
Copy link
Contributor 

bytes.Buffer has a way to create a byte buffer from a given buffer (bytes.NewBuffer) and
a way to take an existing Buffer's []byte back out (bytes.(*Buffer).Bytes).

bufio has neither of those.

In tuning the HTTP server code in https://golang.org/cl/7799047/ I had to go to
great efforts to re-use bufio Readers and Writers, since I couldn't simplify re-use
their underlying buffers.
@bradfitz
Copy link
Contributor Author

Comment 1:

This issue was updated by revision 393b3b1.

R=golang-dev, gri
CC=golang-dev
https://golang.org/cl/8038047

@bradfitz
Copy link
Contributor Author

Comment 2:

Labels changed: added packagechange, garbage.

@bradfitz
Copy link
Contributor Author

Comment 3:

In addition to the hacks in net/http to work around this issue, I also recently did the
same thing for Google's internal sstable code, which was allocating way too much.
I'd like to eventually remove both those hacks.

@bradfitz
Copy link
Contributor Author

Comment 4:

One idea that would help a lot with rapid creation of bufio.NewReader, without changing
any API:
bufio: make Reader backing buffers transient
https://golang.org/cl/8819049
Share garbage between different bufio Readers. When a Reader
has zero buffered data, put its buffer into a pool.

@dvyukov
Copy link
Member

dvyukov commented Apr 28, 2013

Comment 5:

One more use case for sync.Pool, right?

@bradfitz
Copy link
Contributor Author

Comment 6:

This issue was updated by revision b25a53a.

R=r
CC=adg, dvyukov, gobot, golang-dev, rogpeppe
https://golang.org/cl/8819049

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 7:

Labels changed: added feature.

@bradfitz
Copy link
Contributor Author

Comment 8:

Owner changed to @bradfitz.

@bradfitz
Copy link
Contributor Author

Comment 9:

This issue was updated by revision 3e38b7f.

Update issue #6086
Remove switchReader, switchWriter, switchReaderPair,
switchWriterPair, etc.
Now it only maintains pools of bufio Readers and Writers, but
uses Reset instead of working around all their
previously-associated state.
Compared to before the bufio Reset change, it's the same number of
allocations, and also faster:
benchmark                                   old ns/op    new ns/op    delta
BenchmarkClientServer                          111218       109828   -1.25%
BenchmarkClientServerParallel4                  70580        70013   -0.80%
BenchmarkClientServerParallel64                 72636        68919   -5.12%
BenchmarkServer                                139858       137068   -1.99%
BenchmarkServerFakeConnNoKeepAlive              14619        14314   -2.09%
BenchmarkServerFakeConnWithKeepAlive            12390        11361   -8.31%
BenchmarkServerFakeConnWithKeepAliveLite         7630         7306   -4.25%
BenchmarkServerHandlerTypeLen                    9688         9342   -3.57%
BenchmarkServerHandlerNoLen                      8700         8470   -2.64%
BenchmarkServerHandlerNoType                     9255         8949   -3.31%
BenchmarkServerHandlerNoHeader                   7058         6806   -3.57%
benchmark                                  old allocs   new allocs    delta
BenchmarkClientServer                              61           61    0.00%
BenchmarkClientServerParallel4                     61           61    0.00%
BenchmarkClientServerParallel64                    61           61    0.00%
BenchmarkServer                                    16           16    0.00%
BenchmarkServerFakeConnNoKeepAlive                 24           24    0.00%
BenchmarkServerFakeConnWithKeepAlive               19           19    0.00%
BenchmarkServerFakeConnWithKeepAliveLite            9            9    0.00%
BenchmarkServerHandlerTypeLen                      17           17    0.00%
BenchmarkServerHandlerNoLen                        14           14    0.00%
BenchmarkServerHandlerNoType                       15           15    0.00%
BenchmarkServerHandlerNoHeader                      9            9    0.00%
benchmark                                   old bytes    new bytes    delta
BenchmarkClientServer                            6988         6985   -0.04%
BenchmarkClientServerParallel4                   6979         6985    0.09%
BenchmarkClientServerParallel64                  7002         7019    0.24%
BenchmarkServer                                  1846         1848    0.11%
BenchmarkServerFakeConnNoKeepAlive               2420         2412   -0.33%
BenchmarkServerFakeConnWithKeepAlive             2126         2129    0.14%
BenchmarkServerFakeConnWithKeepAliveLite          989          990    0.10%
BenchmarkServerHandlerTypeLen                    1818         1819    0.06%
BenchmarkServerHandlerNoLen                      1775         1777    0.11%
BenchmarkServerHandlerNoType                     1783         1785    0.11%
BenchmarkServerHandlerNoHeader                    989          990    0.10%
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/12708046

@bradfitz
Copy link
Contributor Author

Comment 10:

We have bufio.{Reader,Writer}.Reset now.  I think that's as much as we're going to do on
this bug.

Status changed to Fixed.

@bradfitz bradfitz self-assigned this Aug 16, 2013
@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GarbageCollector
Projects
None yet
Milestone
Go1.2
Development

No branches or pull requests
4 participants
@bradfitz @rsc @dvyukov @gopherbot