Expose Buffer pools to third-party encoders #376
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
prashantv
approved these changes
Mar 14, 2017
buffer/pool.go
Outdated
| import "sync" | ||
|
|
||
| // A Pool is a type-safe wrapper around a sync.Pool. | ||
| type Pool sync.Pool |
There was a problem hiding this comment.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
reference to a buffer.
know how to return them to their pools.
This PR introduces a
buffer.Pooltype to solve this problem. Individualbuffers 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.