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

Don't allocate a new buffer pool per connection. #4397

Closed
Stebalien opened this issue Nov 17, 2017 · 3 comments
Closed

Don't allocate a new buffer pool per connection. #4397

Stebalien opened this issue Nov 17, 2017 · 3 comments
Assignees
Labels
P0 Critical: Tackled by core team ASAP

Comments

@Stebalien
Copy link
Member

It looks like we were doing this in secio.

Secio PR: libp2p/go-libp2p-secio#21

Related, pull through the updated go-msgio/mpool. It no longer uses a global lock...

@Stebalien Stebalien added the P0 Critical: Tackled by core team ASAP label Nov 17, 2017
@Stebalien Stebalien self-assigned this Nov 17, 2017
@Stebalien
Copy link
Member Author

On further inspection, it we may have been sharing some buffer pools in some cases as some packages copied the buffer pools while others referenced them (which would have initialized the inner buffer pools). Regardless, this behavior was buggy as hell, prone to deadlocks (copying initialized locks...), and inefficient.

@Kubuxu
Copy link
Member

Kubuxu commented Nov 21, 2017

I think this should be fixed by changing https://github.com/libp2p/go-msgio/blob/master/mpool/pool.go#L29 to be a pointer to a Pool.

This way simple re-referencing would not copy the locks.

@Stebalien
Copy link
Member Author

This is actually fixed in master, I just stopped copying around the pool. However, yes, storing this as a reference would probably be a good idea and would prevent things like this from coming up in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Critical: Tackled by core team ASAP
Projects
None yet
Development

No branches or pull requests

2 participants