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

Expose Buffer pools to third-party encoders #376

Merged
merged 2 commits into from
Mar 14, 2017
Merged

Expose Buffer pools to third-party encoders #376

merged 2 commits into from
Mar 14, 2017

Conversation

akshayjshah
Copy link
Contributor

Zap currently uses an internal package to house its buffer pool, and it returns
all buffers to that pool. This is problematic for two reasons:

  1. Third-party encoders can easily corrupt the pool by keeping a
    reference to a buffer.
  2. Third-party encoders can't pool their buffers, since zap needs to
    know how to return them to their pools.

This PR introduces a buffer.Pool type to solve this problem. Individual
buffers can only be created via a pool, and each buffer keeps a reference to its
pool of origin. This allows zap to continue using a single internal pool, but it
allows third-party encoders to safely pool their buffers too.

Zap currently uses an internal package to house its buffer pool, and it returns
all buffers to that pool. This is problematic for two reasons:

1. First, third-party encoders can easily corrupt the pool by keeping a
   reference to a buffer.
2. Second, third-party encoders can't pool their buffers, since zap needs to
   know how to return them to their pools.

This PR introduces a `buffer.Pool` type to solve this problem. Individual
buffers can only be created via a pool, and each buffer keeps a reference to its
pool of origin. This allows zap to continue using a single internal pool, but it
allows third-party encoders to safely pool their buffers too.
buffer/pool.go Outdated
import "sync"

// A Pool is a type-safe wrapper around a sync.Pool.
type Pool sync.Pool
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think this code will be cleaner if we make this a struct with a sync.Pool as a field. Retyping is useful for slices and maps since it lets callers create them more easily, but there's no advantage here, and we don't want users to get access to the underlying sync.Pool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, can do.

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

Successfully merging this pull request may close these issues.

2 participants