Address feedback

metric/unaggregated/types.go

@@ -90,9 +90,9 @@ type GaugeWithPolicies struct {
 
 // MetricUnion is a union of different types of metrics, only one of which is valid
 // at any given time. The actual type of the metric depends on the type field,
-// which determines which value field is valid. We intentionally do not use value
-// pointers and nil checks to determine which type is valid in order to avoid the GC
-// overhead of marking and sweeping the metrics.
+// which determines which value field is valid. Note that if the timer values are
+// allocated from a pool, the TimerValPool should be set to the originating pool,
+// and the caller is responsible for returning the timer values to the pool.
 // NB(xichen): possibly use refcounting to replace explicit ownership tracking.
 type MetricUnion struct {
 	Type          Type

protocol/msgpack/unaggregated_iterator.go

@@ -23,13 +23,18 @@ package msgpack
 import (
 	"fmt"
 	"io"
+	"math"
 
 	"github.com/m3db/m3metrics/metric"
 	"github.com/m3db/m3metrics/metric/unaggregated"
 	"github.com/m3db/m3metrics/policy"
 	"github.com/m3db/m3x/pool"
 )
 
+const (
+	defaultInitTimerValuesCapacity = 16
+)
+
 // unaggregatedIterator uses MessagePack to decode different types of unaggregated metrics.
 // It is not thread-safe.
 type unaggregatedIterator struct {
@@ -59,6 +64,7 @@ func NewUnaggregatedIterator(reader io.Reader, opts UnaggregatedIteratorOptions)
 		largeFloatsSize:     opts.LargeFloatsSize(),
 		largeFloatsPool:     opts.LargeFloatsPool(),
 		iteratorPool:        opts.IteratorPool(),
+		timerValues:         make([]float64, 0, defaultInitTimerValuesCapacity),
 	}
 	return it
 }
@@ -182,7 +188,11 @@ func (it *unaggregatedIterator) decodeBatchTimer() {
 		it.timerValues = it.timerValues[:0]
 		timerValues = it.timerValues
 	} else if numValues <= it.largeFloatsSize {
-		it.timerValues = make([]float64, 0, numValues)
+		newCapcity := int(math.Max(float64(numValues), float64(cap(it.timerValues)*2)))
+		if newCapcity > it.largeFloatsSize {
+			newCapcity = it.largeFloatsSize
+		}
+		it.timerValues = make([]float64, 0, newCapcity)
 		timerValues = it.timerValues
 	} else {
 		timerValues = it.largeFloatsPool.Get(numValues)

protocol/msgpack/unaggregated_iterator_test.go

@@ -236,13 +236,14 @@ func TestUnaggregatedIteratorDecodeBatchTimerWithAllocPoolAlloc(t *testing.T) {
 	enc := testUnaggregatedEncoder(t).(*unaggregatedEncoder)
 	bt := unaggregated.BatchTimer{
 		ID:     []byte("foo"),
-		Values: []float64{1.0, 2.0, 3.0, 4.0},
+		Values: []float64{1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0},
 	}
 	enc.encodeBatchTimer(bt)
 	require.NoError(t, enc.err())
 
 	// Allocate a large enough buffer to avoid triggering an allocation.
 	it := testUnaggregatedIterator(t, enc.Encoder().Buffer()).(*unaggregatedIterator)
+	it.timerValues = nil
 	it.largeFloatsSize = 2
 	it.decodeBatchTimer()