[query] Fix a bug with step iteration allocating #2185
Conversation
| @@ -63,7 +63,7 @@ func NewStepLookbackConsolidator( | |||
| startTime time.Time, | |||
| fn ConsolidationFunc, | |||
| ) *StepLookbackConsolidator { | |||
| datapoints := make([]ts.Datapoint, 0, initLength) | |||
| datapoints := make([]ts.Datapoint, 0, initLength*BufferSteps) | |||
There was a problem hiding this comment.
Hm, should this perhaps just be BufferSteps instead of initLength*BufferSteps?
From my current perhaps limited understanding we should only ever be buffering up to BufferSteps max in c.unconsumed so having 320 values instead of 32 values (initLength=10, BufferSteps=32) able to be buffered might be a bit of a waste given *StepLookbackConsolidator should not need more than 32?
| @@ -108,6 +108,7 @@ func (c *StepLookbackConsolidator) ConsolidateAndMoveToNext() float64 { | |||
| } | |||
|
|
|||
| val := c.unconsumed[0] | |||
| c.unconsumed = c.unconsumed[1:] | |||
| copy(c.unconsumed, c.unconsumed[1:]) | |||
There was a problem hiding this comment.
We also want to make this change in step_accumulator right?
src/query/pools/query_pools.go
Outdated
| // iteratorPoolSize = 1000 | ||
| // checkedBytesWrapperPoolSize = 128 | ||
| // defaultIdentifierPoolSize = 128 |
| // defaultIdentifierPoolSize = 128 | ||
|
|
||
| // BuildIteratorPoolsOptions is a set of build iterator pools. | ||
| type BuildIteratorPoolsOptions struct { |
There was a problem hiding this comment.
nit: maybe better to take in these values as a constructor rather than exposing the internal variables for mutation?
robskillington
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2185 +/- ##
======================================
Coverage 72.2% 72.2%
======================================
Files 1019 1019
Lines 88946 88946
======================================
Hits 64278 64278
Misses 20373 20373
Partials 4295 4295
Continue to review full report at Codecov.
|
No description provided.