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

use buffer pool for stream buffers #5

Merged
merged 1 commit into from
Sep 25, 2018
Merged

use buffer pool for stream buffers #5

merged 1 commit into from
Sep 25, 2018

Conversation

Stebalien
Copy link
Collaborator

  1. Reduces GC pressure when creating/destroying many streams.
  2. Frees memory when not in use.

This doesn't noticeably hurt throughput.

@Stebalien
Copy link
Collaborator Author

This uses https://github.com/libp2p/go-buffer-pool. The Buffer implementation is modeled after go's bytes.Buffer and passes go's test cases (with some small modifications).

@Stebalien
Copy link
Collaborator Author

After some testing, I've noticed that this does some really nice things for go-ipfs memory usage under load.

@jvsteiner
Copy link

jvsteiner commented Mar 10, 2018

Anecdotally, I've noticed about 50-70% memory usage reduction in my non-ipfs application.

@jvsteiner
Copy link

what about this one? it's cool...

@Stebalien
Copy link
Collaborator Author

@jvsteiner I need to think about it a bit. Go's buffer implementation avoids shrinking buffers unless something tries to read from an empty buffer. It uses that as an "idle" signal. Unfortunately, that doesn't kick in in yamux because yamux remembers how much data is in the buffer and avoids reading.

However, my implementation shrinks the buffer if the amount of data in it shrinks by 8x capacity. I'd like to test:

  1. Having yamux intentionally read from the empty buffer (instead of checking Len() == 0) to see how that performs compared to this library (both in terms of GC pressure and memory usage).
  2. Using the same "shrink on empty read" strategy in my custom Buffer.

1. Reduces GC pressure when creating/destroying many streams.
2. Frees memory when not in use.

This doesn't noticeably hurt throughput.
@Stebalien
Copy link
Collaborator Author

I've tried the same "shrink on empty read" strategy and it really doesn't make a difference. This works and has great memory savings so I'm going to merge it. We can always revisit it later if we run into trouble.

@Stebalien Stebalien merged commit 3f61f99 into master Sep 25, 2018
@Stebalien Stebalien deleted the feat/buffer-pool branch September 25, 2018 22:11
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

Successfully merging this pull request may close these issues.

2 participants