Skip to content

Commit 018da15

Browse files
authored
core: avoid per block allocation in aggregation (#1973)
The inner loop of MemoryDatabase.executeImpl runs once per block per matched series, so for a broad query it can execute billions of times. Three objects were being allocated on every iteration: * The `newBuffer` argument was a local def. Passing a def where a function is expected eta-expands at the call site, which is inside the loop, so the captured cfStep/bufStart/bufEnd produced a new function object per block. Bind it to a val once per query instead. * The inner foreach body ended with the Int returned by add(), so the loop body was typed as AnyVal and boxed a new Integer per block. The value is buffer.values.length, well above the Integer cache limit, so none of these were shared. Keep the body Unit typed. * AggregateCollector.add iterated its blocks with foreach, allocating a closure that captured the buffer, op, and the aggr/cf/multiple params. Use an explicit loop over the list in both implementations. After this the only remaining allocation in the per block frame is the `List(b)` wrapper. Removing that means adding a single block overload to the public AggregateCollector trait, left for a separate change. Note that the four `Math.addNaN _` style eta-expansions in add() are NOT a source of allocation, despite looking like one in a profile: Math is a static module, so they compile to non-capturing invokedynamic call sites that are bound to a singleton and are already specialized.
1 parent 25c9e64 commit 018da15

2 files changed

Lines changed: 20 additions & 5 deletions

File tree

atlas-core/src/main/scala/com/netflix/atlas/core/db/AggregateCollector.scala

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,12 @@ abstract class SimpleAggregateCollector extends AggregateCollector {
121121
case Block.Max => Math.maxNaN _
122122
}
123123

124-
blocks.foreach { b =>
124+
// Explicit loop rather than `foreach`: the closure would capture the buffer, op,
125+
// and the aggr/cf/multiple params, allocating a new object on every call. This is
126+
// called once per block, so it shows up as a top allocation leaf.
127+
var bs = blocks
128+
while (bs.nonEmpty) {
129+
val b = bs.head
125130
if (valueMask != null) {
126131
val v = buffer.aggrBlock(b, aggr, ConsolidationFunction.Sum, multiple, op)
127132
buffer.valueMask(valueMask, b, multiple)
@@ -130,6 +135,7 @@ abstract class SimpleAggregateCollector extends AggregateCollector {
130135
val v = buffer.aggrBlock(b, aggr, cf, multiple, op)
131136
valueCount += v
132137
}
138+
bs = bs.tail
133139
}
134140
buffer.values.length
135141
}
@@ -290,9 +296,12 @@ class AllAggregateCollector extends LimitedAggregateCollector {
290296
case Block.Max => Math.maxNaN _
291297
}
292298

293-
blocks.foreach { b =>
294-
val v = buffer.aggrBlock(b, aggr, cf, multiple, op)
295-
valueCount += v
299+
// Explicit loop to avoid allocating a capturing closure per call, see the comment
300+
// on SimpleAggregateCollector.add.
301+
var bs = blocks
302+
while (bs.nonEmpty) {
303+
valueCount += buffer.aggrBlock(bs.head, aggr, cf, multiple, op)
304+
bs = bs.tail
296305
}
297306

298307
if (valueCount > 0) add(buffer)

atlas-core/src/main/scala/com/netflix/atlas/core/db/MemoryDatabase.scala

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,10 @@ class MemoryDatabase(registry: Registry, config: Config) extends Database {
229229
val bufStart = context.start
230230
val bufEnd = context.end - cfStep
231231

232-
def newBuffer(tags: Map[String, String]): TimeSeriesBuffer = {
232+
// Bound to a val rather than a local def: passing a def where a function is expected
233+
// eta-expands at the call site, and since this captures cfStep/bufStart/bufEnd that
234+
// would allocate a new function object for every block. Bind it once per query.
235+
val newBuffer: Map[String, String] => TimeSeriesBuffer = { tags =>
233236
TimeSeriesBuffer(tags, cfStep, bufStart, bufEnd)
234237
}
235238

@@ -249,6 +252,9 @@ class MemoryDatabase(registry: Registry, config: Config) extends Database {
249252
if (b.start <= bufEnd && blockEnd > bufStart - cfStep) {
250253
numAggrBlocks += 1
251254
collector.add(item.tags, List(b), aggr, expr.cf, multiple, newBuffer)
255+
// Keep the closure Unit typed. Otherwise the Int result of add() becomes the
256+
// value of the loop body and gets boxed into an Integer for every block.
257+
()
252258
}
253259
}
254260
}

0 commit comments

Comments
 (0)