FrankChen021 opened a new pull request, #5283: URL: https://github.com/apache/calcite/pull/5283
## Jira Link [CALCITE-7805](https://issues.apache.org/jira/browse/CALCITE-7805) ## Changes Proposed `RexShuttle` and `SqlShuttle` are transforming visitors: they visit every child and return either the original child or a replacement. Before this change, both visitors copied operand collections before knowing whether any child would change. The previous behavior was effectively: ```text Visit operands ↓ Always allocate a new operand array/list ↓ Copy every operand ↓ Discover that nothing changed ↓ Return the original node anyway ``` This PR makes the operand collections copy-on-write: ```text Visit operands without copying ↓ Did any child return a different instance? ├─ No → reuse the existing operand collection └─ Yes → allocate at the first change and copy the operands ``` For `RexShuttle.visitList`, the immutable-list builder is created only when the first changed child is encountered. An unchanged `ImmutableList` is returned directly; an unchanged non-immutable input is still copied to preserve the method's existing immutable-result contract. For `SqlShuttle`, the operand array is created only when the first changed child is encountered. Callers that explicitly request `alwaysCopy` retain the existing unconditional-copy behavior. This does not skip traversal, validation, type inference, or conversion. It only avoids temporary operand collections when a traversal leaves the children unchanged. This branch is based directly on `main` at `38413ece6` and is independent of the other allocation optimizations. ## Path Exercised by Druid The Druid string-IN planning benchmark exercises both changed paths. Before SQL-to-Rel conversion, the validated SQL contains the large `SCALAR_IN_ARRAY(..., ARRAY[...])` expression produced from the input `IN` list. JFR showed read-mostly SQL-tree traversals through: ```text InPlanningBenchmark.queryStringInSqlPlanOnly ↓ DruidPlanner.validate / QueryHandler.validate ↓ SqlResourceCollectorShuttle ↓ SqlShuttle ``` `SqlResourceCollectorShuttle` examines the tree to collect referenced resources but normally does not replace the literal operands. The previous implementation still copied each visited call's operands into a new `SqlNode[]`. The same unnecessary copying also occurred while `SqlValidatorImpl.getParameterRowType` traversed the query. After SQL-to-Rel conversion, JFR showed the Rex path through: ```text SqlToRelConverter.flattenTypes ↓ RelStructuredTypeFlattener ↓ RewriteRexShuttle ↓ LogicalFilter ↓ RexShuttle.visitList ``` The large array's literal operands generally remain the same objects during this traversal. The previous implementation nevertheless built a replacement `ImmutableList`; this PR reuses the existing immutable operand list when no child changes. ## Isolated Benchmark Benchmark: Druid `InPlanningBenchmark.queryStringInSqlPlanOnly`, generating the SQL string literals `'1'` through `'N'`. The comparison changes only Calcite from the PR base (`38413ece6`) to this PR (`8ed69e214`). Both variants use matching core and linq4j artifacts, the same Druid benchmark classes, and the following JMH settings: - `inSubQueryThreshold=2147483647` - `rowsPerSegment=500000` - 2 warmup iterations and 5 measurement iterations, 1 second each - 2 forks - `-prof gc` | String literals | Base time | PR time | Time change | Base allocation | PR allocation | Allocation reduction | | ---: | ---: | ---: | ---: | ---: | ---: | ---: | | 100,000 | 740.60 ms/op | 739.56 ms/op | -0.14% | 1.534 GB/op | 1.492 GB/op | 2.72% | | 1,000,000 | 7,832.82 ms/op | 7,895.67 ms/op | +0.80% | 15.447 GB/op | 15.207 GB/op | 1.55% | The execution-time error ranges overlap, so this isolated run does not establish a time improvement. It does show lower allocation at both sizes. Allocation is total bytes allocated during one planning operation, not retained memory or peak heap usage. ## Validation - `./gradlew :core:compileJava :core:compileTestJava` - `./gradlew :core:test --tests org.apache.calcite.test.RexShuttleTest` (2 completed, 0 failed) - `./gradlew :core:autostyleJavaCheck :core:checkstyleAll` -- 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]
