From 8f5f85e36f9bc6c16890610e40ec3b1e3762510c Mon Sep 17 00:00:00 2001 From: Dieter Plaetinck Date: Thu, 1 Oct 2020 21:32:14 +0200 Subject: [PATCH 1/3] pointslicepool: use size classes 1) save memory by not using large slices when we only need small ones In particular, fix #1912 2) use a default size that corresponds to a capacity that append() actually uses (I believe a size of 800 gets rounded up by make) --- pointslicepool/pointslicepool.go | 58 +++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/pointslicepool/pointslicepool.go b/pointslicepool/pointslicepool.go index 357641e570..4d206d6e03 100644 --- a/pointslicepool/pointslicepool.go +++ b/pointslicepool/pointslicepool.go @@ -6,24 +6,47 @@ import ( "github.com/grafana/metrictank/schema" ) -// default size is probably bigger than what most responses need, but it saves [re]allocations -// also it's possible that occasionally more size is needed, causing a realloc of underlying array, and that extra space will stick around until next GC run. -const DefaultPointSliceSize = 2000 +// DefaultPointSliceSize is the default slice length to pull out of the pool for Get() calls. MUST match a size class. +// because we default maxDataPoints to 800, this seems like a sensible option +// might be worth experimenting with a smaller size class, and/or making classes and the default size configurable. +// i suspect many panels have 200= 0; i-- { + if p.sizes[i] <= len(s) { + p.pools[i].Put(s[:0]) + return + } + } } func (p *PointSlicePool) Get() []schema.Point { @@ -32,18 +55,21 @@ func (p *PointSlicePool) Get() []schema.Point { // GetMin returns a pointslice that has at least minCap capacity func (p *PointSlicePool) GetMin(minCap int) []schema.Point { - candidate, ok := p.p.Get().([]schema.Point) + // find the smallest class that certainly has the right capacity + class := len(p.sizes) - 1 + for i := 0; i < len(p.sizes); i++ { + if p.sizes[i] >= minCap { + class = i + break + } + } + candidate, ok := p.pools[class].Get().([]schema.Point) if ok { + // this check is because the last class may return a slice that is too small if cap(candidate) >= minCap { return candidate } - p.p.Put(candidate) - } - if minCap > p.defaultSize { - return make([]schema.Point, 0, minCap) + p.pools[class].Put(candidate) } - // even if our caller needs a smaller cap now, we expect they will put it back in the pool - // so it can later be reused. - // may as well allocate a size now that we expect will be more useful down the road. - return make([]schema.Point, 0, p.defaultSize) + return make([]schema.Point, 0, minCap) } From fb89cc521a6e3f2b42186f789af1671a79138916 Mon Sep 17 00:00:00 2001 From: Dieter Plaetinck Date: Thu, 1 Oct 2020 22:38:30 +0200 Subject: [PATCH 2/3] accommodate GetMin() with huge minCap e.g. in docker-dev with 1s precision 2days means 172800 points. This is *before* maxdatapoints runtime consolidation, so we may as well accommodate it with some special size classes this makes sure our large slices won't be taken up by smaller (4k) requests --- pointslicepool/pointslicepool.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pointslicepool/pointslicepool.go b/pointslicepool/pointslicepool.go index 4d206d6e03..2c31ebbc20 100644 --- a/pointslicepool/pointslicepool.go +++ b/pointslicepool/pointslicepool.go @@ -17,8 +17,8 @@ const DefaultPointSliceSize = 1024 // internally it has several pools for different size classes type PointSlicePool struct { defaultSize int - sizes [6]int - pools [6]sync.Pool + sizes [8]int + pools [8]sync.Pool } func New(defaultSize int) *PointSlicePool { @@ -33,7 +33,7 @@ func New(defaultSize int) *PointSlicePool { // too many classes means you may allocate needlessly much (we could have useful slices, but they're in a higher size class) // perhaps the ideal is many finegrained classes, and upon GetMin(), try multiple classes as needed // we can also think about dynamically constructing size classes based on the real lengths/mincapacities we see at runtime - sizes: [6]int{0, 32, 128, 1024, 2048, 4096}, + sizes: [8]int{0, 32, 128, 1024, 4096, 32768, 262144, 2097152}, } return &p From e54a74ef6a8e82b29c485b6ea16bd5a52c3fdcaf Mon Sep 17 00:00:00 2001 From: Dieter Plaetinck Date: Fri, 2 Oct 2020 11:04:40 +0200 Subject: [PATCH 3/3] fix: use cap, not len --- pointslicepool/pointslicepool.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pointslicepool/pointslicepool.go b/pointslicepool/pointslicepool.go index 2c31ebbc20..458441d6aa 100644 --- a/pointslicepool/pointslicepool.go +++ b/pointslicepool/pointslicepool.go @@ -25,14 +25,14 @@ func New(defaultSize int) *PointSlicePool { p := PointSlicePool{ defaultSize: defaultSize, - // these are lower bounds. Every class contains slices of the exact bound length, or larger. Thus: + // these are lower bounds. Every class contains slices of the exact bound capacity, or larger. Thus: // * On Get, the right class is the smallest size that is larger or equal than the given slice // * On Put, the right class for a given slice is the largest size that is equal or smaller than the given slice // why these? they're just a first stab at it. // too few size classes and Get() returns needlessly large slices // too many classes means you may allocate needlessly much (we could have useful slices, but they're in a higher size class) // perhaps the ideal is many finegrained classes, and upon GetMin(), try multiple classes as needed - // we can also think about dynamically constructing size classes based on the real lengths/mincapacities we see at runtime + // we can also think about dynamically constructing size classes based on the real capacity/mincapacities we see at runtime sizes: [8]int{0, 32, 128, 1024, 4096, 32768, 262144, 2097152}, } @@ -42,7 +42,7 @@ func New(defaultSize int) *PointSlicePool { // Put puts the the slice in the appropriate size class func (p *PointSlicePool) Put(s []schema.Point) { for i := len(p.sizes) - 1; i >= 0; i-- { - if p.sizes[i] <= len(s) { + if p.sizes[i] <= cap(s) { p.pools[i].Put(s[:0]) return }