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.

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

[FEATURE] eliminate buffer pool example #845

Open
ghost opened this issue Aug 17, 2023 · 1 comment
Open

[FEATURE] eliminate buffer pool example #845

ghost opened this issue Aug 17, 2023 · 1 comment

Comments

@ghost
Copy link

ghost commented Aug 17, 2023

Is your feature request related to a problem? Please describe.

Remove the bufferpool example. The example's commit message says:

This usage will cause the 8k buffer(4k read buffer + 4k write buffer) allocated by net.http can't be GC(Observed by heap profiling, see picture below) . The purpose of saving memory is not achieved even if the WriteBufferPool is used.

Returning from the handler does allow the GC to recover memory referenced by net/http functions the stack, but that's independent of the memory savings enabled by the write buffer pool.

The write buffer pool shares buffers across multiple connections instead of allocating a write buffer per connection. The memory savings from sharing increases as the write buffer size increases. Because the write buffer size in the example is small compared to the other memory allocated per connection, the pool does not achieve a large memory saving.

Let's move on from the example's commit message. In the absence of a README describing the purpose of the example, one might try derive the purpose of the example from the directory name, bufferpool. This is is not a good example of the write buffer pool feature because the example does not write to the connection and the write buffer size is small.

The file client.go is basically a load testing client used to support the claim in the commit message. The load testing code is distracting to somebody wanting to learn how to use the package.

Describe the solution that you would like.

The example does illustrate a good point -- the application can reduce memory use by returning from the HTTP handler function after starting goroutines to read and write messages. Rather than improving the example to make the point clear (add a README, remove the load tester, rename directory, remove the buffer pool), I think it's better to improve the other examples.

The chat example example does return from the handler after starting goroutines to read and write the connection. The command, echo and filewatch examples should be updated to follow this best practice.

@lxzan
Copy link

lxzan commented Aug 18, 2023

The solution to this problem is also simple.

  1. Do not reuse the bufio.ReadWriter returned by hijack.
  2. Start a new goroutine when reading messages in a loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant