Skip to content
This repository has been archived by the owner on Oct 17, 2018. It is now read-only.

Commit

Permalink
[buffered encoder pool] Address comments on PR
Browse files Browse the repository at this point in the history
  • Loading branch information
Jerome Froelich committed Aug 28, 2017
1 parent c023826 commit c373cff
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 22 deletions.
10 changes: 5 additions & 5 deletions protocol/msgpack/buffered_encoder_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ package msgpack
import "github.com/m3db/m3x/pool"

type bufferedEncoderPool struct {
maxBufferCapacity int
pool pool.ObjectPool
maxCapacity int64
pool pool.ObjectPool
}

// NewBufferedEncoderPool creates a new pool for buffered encoders.
func NewBufferedEncoderPool(opts BufferedEncoderPoolOptions) BufferedEncoderPool {
return &bufferedEncoderPool{
maxBufferCapacity: opts.MaxBufferCapacity(),
pool: pool.NewObjectPool(opts.ObjectPoolOptions()),
maxCapacity: opts.MaxCapacity(),
pool: pool.NewObjectPool(opts.ObjectPoolOptions()),
}
}

Expand All @@ -46,7 +46,7 @@ func (p *bufferedEncoderPool) Get() BufferedEncoder {
}

func (p *bufferedEncoderPool) Put(encoder BufferedEncoder) {
if p.maxBufferCapacity != 0 && cap(encoder.Buffer().Bytes()) > p.maxBufferCapacity {
if int64(encoder.Buffer().Cap()) > p.maxCapacity {
return
}
p.pool.Put(encoder)
Expand Down
6 changes: 2 additions & 4 deletions protocol/msgpack/buffered_encoder_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package msgpack

import (
"fmt"
"testing"

"github.com/m3db/m3x/pool"
Expand Down Expand Up @@ -59,7 +58,7 @@ func TestBufferedEncoderPool(t *testing.T) {
func TestBufferedEncoderPoolMaxCapacity(t *testing.T) {
poolOpts := pool.NewObjectPoolOptions().SetSize(1)
opts := NewBufferedEncoderPoolOptions().
SetMaxBufferCapacity(2).
SetMaxCapacity(2).
SetObjectPoolOptions(poolOpts)

p := NewBufferedEncoderPool(opts)
Expand All @@ -78,6 +77,5 @@ func TestBufferedEncoderPoolMaxCapacity(t *testing.T) {
// Retrieve an encoder and assert it's a different encoder since
// the previous one exceeded the maximum capacity of the pool.
encoder = p.Get()
fmt.Println(encoder.Buffer().Len())
require.Equal(t, 0, encoder.Buffer().Len())
require.Equal(t, 0, encoder.Buffer().Cap())
}
19 changes: 10 additions & 9 deletions protocol/msgpack/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@
package msgpack

import xpool "github.com/m3db/m3x/pool"
import "math"

const (
// The maximum capacity of buffers that can be returned to the buffered
// encoder pool. A value of 0 indicates no maximum so all buffered
// encoders will be returned to the pool.
defaultBufferedEncoderPoolMaxCapacity = 0
defaultBufferedEncoderPoolMaxCapacity = math.MaxInt64

// Whether the iterator should ignore higher-than-supported version
// by default for unaggregated iterator.
Expand All @@ -48,26 +49,26 @@ const (
)

type bufferedEncoderPoolOptions struct {
maxBufferCapacity int
poolOpts xpool.ObjectPoolOptions
maxCapacity int64
poolOpts xpool.ObjectPoolOptions
}

// NewBufferedEncoderPoolOptions creates a new set of buffered encoder pool options.
func NewBufferedEncoderPoolOptions() BufferedEncoderPoolOptions {
return &bufferedEncoderPoolOptions{
maxBufferCapacity: defaultBufferedEncoderPoolMaxCapacity,
poolOpts: xpool.NewObjectPoolOptions(),
maxCapacity: defaultBufferedEncoderPoolMaxCapacity,
poolOpts: xpool.NewObjectPoolOptions(),
}
}

func (o *bufferedEncoderPoolOptions) SetMaxBufferCapacity(value int) BufferedEncoderPoolOptions {
func (o *bufferedEncoderPoolOptions) SetMaxCapacity(value int64) BufferedEncoderPoolOptions {
opts := *o
opts.maxBufferCapacity = value
opts.maxCapacity = value
return &opts
}

func (o *bufferedEncoderPoolOptions) MaxBufferCapacity() int {
return o.maxBufferCapacity
func (o *bufferedEncoderPoolOptions) MaxCapacity() int64 {
return o.maxCapacity
}

func (o *bufferedEncoderPoolOptions) SetObjectPoolOptions(value xpool.ObjectPoolOptions) BufferedEncoderPoolOptions {
Expand Down
8 changes: 4 additions & 4 deletions protocol/msgpack/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ type BufferedEncoderPool interface {

// BufferedEncoderPoolOptions provides options for buffered encoder pools.
type BufferedEncoderPoolOptions interface {
// SetMaxBufferCapacity sets the maximum buffer capacity.
SetMaxBufferCapacity(value int) BufferedEncoderPoolOptions
// SetMaxCapacity sets the maximum capacity of buffers that can be returned to the pool.
SetMaxCapacity(value int64) BufferedEncoderPoolOptions

// MaxBufferCapacity returns the maximum buffer capacity.
MaxBufferCapacity() int
// MaxBufferCapacity returns the maximum capacity of buffers that can be returned to the pool.
MaxCapacity() int64

// SetObjectPoolOptions sets the object pool options.
SetObjectPoolOptions(value pool.ObjectPoolOptions) BufferedEncoderPoolOptions
Expand Down

0 comments on commit c373cff

Please sign in to comment.