alex-plekhanov commented on code in PR #12096:
URL: https://github.com/apache/ignite/pull/12096#discussion_r3348420114
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/window/WindowPartitionBase.java:
##########
@@ -64,4 +73,16 @@ protected final Row
createResultRow(RowHandler.RowFactory<Row> rowFactory, Row s
Row resultsRow = this.rowFactory.create(results);
return rowFactory.handler().concat(src, resultsRow);
}
+
+ /** Adds row to memory tracker. */
+ protected final void onRowAdded(Row row) {
Review Comment:
Only used by BufferingWindowPartition, can be moved to these class.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/window/WindowFunctionFactory.java:
##########
@@ -50,12 +50,11 @@ final class WindowFunctionFactory<Row> extends
AccumulatorsFactoryBase<Row> {
this.inputRowType = inputRowType;
ImmutableList.Builder<WindowFunctionPrototype<Row>> prototypes =
ImmutableList.builder();
- for (AggregateCall aggCall : aggCalls) {
+ for (AggregateCall aggCall : aggCalls)
Review Comment:
Braces are not needed only for one line statement, here braces are required.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/window/BufferingWindowPartition.java:
##########
@@ -76,14 +77,15 @@ final class BufferingWindowPartition<Row> extends
WindowPartitionBase<Row> {
}
Row resultRow = createResultRow(factory, currRow, accResults);
- output.add(resultRow);
+ output.accept(resultRow);
prevRow = currRow;
}
}
/** {@inheritDoc} */
@Override public void reset() {
+ buf.forEach(this::onRowRemoved);
Review Comment:
It's not very good to calculate size of object 4 times (on add to partition,
on remove from partition, on add to node buffer, on remove from node buffer),
since it can be resource consuming (in case of complex objects reflection is
used). But let's fix it together with WindowNode refactoring. If node buffer
become constant-size, we can omit rows memory tracking for node and track rows
memory only by window partition. In window partition we can reset memory
tracker in this method (instead of rows iterating)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
