[
https://issues.apache.org/jira/browse/CALCITE-7805?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18118224#comment-18118224
]
Frank Chen commented on CALCITE-7805:
-------------------------------------
[~julianhyde] I would like to clarify that these text ARE generated by AI but
the underlying problems are real and have impact on us, and changes are
carefully reviewed and the outcome is substantial. I do not consider “slop” to
be specific or constructive technical feedback.
I composed an email to address your concerns separately in the dev channel.
> Reduce temporary object allocation when validating large string ARRAYs to
> lower GC pressure
> -------------------------------------------------------------------------------------------
>
> Key: CALCITE-7805
> URL: https://issues.apache.org/jira/browse/CALCITE-7805
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Affects Versions: 1.42.0
> Reporter: Frank Chen
> Priority: Major
> Labels: pull-request-available
>
> CALCITE-7782 and [PR #5263|https://github.com/apache/calcite/pull/5263]
> eliminated quadratic operand-list copying while validating large ARRAY and
> MAP constructors. That correction allows a 1,000,000-element string ARRAY to
> complete instead of running for more than 20 minutes.
> After that correction, validation is linear, but planning still creates many
> temporary objects for every string literal. In the benchmark below, planning
> a 1,000,000-element string IN expression allocates approximately 15.4 GB
> during one operation and takes approximately 7.8 seconds.
> The allocation values below are cumulative allocation during one planning
> operation, not retained memory or peak heap usage.
> h2. Reproducer
> Apache Druid exposes the remaining allocation through a large string IN
> predicate:
> {code:sql}
> SELECT string1
> FROM foo
> WHERE string1 IN ('1', '2', ..., '1000000')
> {code}
> Druid rewrites the predicate to an expression containing a string ARRAY:
> {code}
> SCALAR_IN_ARRAY(string1, ARRAY['1', '2', ..., '1000000'])
> {code}
> The benchmark is {{InPlanningBenchmark.queryStringInSqlPlanOnly}}.
> Parameters:
> * {{inSubQueryThreshold = 2147483647}}
> * {{rowsPerSegment = 500000}}
> * 2 JMH forks
> * 2 one-second warmup iterations per fork
> * 5 one-second measurement iterations per fork
> * GC profiler enabled
> * Temurin 25.0.4.1
> The Calcite baseline is commit {{38413ece6}}, which includes the merged
> CALCITE-7782 change.
> h2. Remaining allocation sources
> Profiling identified five independent allocation paths.
> h3. 1. Generated CAST calls repeat operator resolution
> ARRAY validation creates generated CAST calls when an operand type differs
> from the common element type. The CAST node already contains the built-in
> CAST operator, but its validated type is not registered immediately. A later
> {{deriveType}} call constructs temporary operand and argument-type lists and
> searches the operator table for the same CAST operator.
> The generated CAST can instead be validated directly through its existing
> operator, preserving operand validation and validated-type registration while
> avoiding redundant routine lookup.
> h3. 2. Unchanged charset and collation create equivalent types
> During least-restrictive character-type inference,
> {{BasicSqlType.createWithCharsetAndCollation}} may be called with the charset
> and collation already present on the immutable type. The current
> implementation creates another {{BasicSqlType}} and charset wrapper before
> canonicalization returns the original canonical type. The method can return
> the same instance when these attributes are unchanged.
> h3. 3. Canonicalization occurs after temporary type construction
> {{RelDataTypeFactoryImpl}} has a global weak canonical-type interner, but it
> accepts an already constructed type. It prevents duplicate types from being
> retained but does not prevent temporary {{BasicSqlType}}, charset wrapper,
> and digest allocation.
> A small bounded front cache can reuse the most recently created base and
> decorated type for each {{SqlTypeName}} before constructing another type. The
> existing global weak interner remains authoritative.
> h3. 4. SQL and Rex visitors eagerly copy unchanged operands
> {{SqlShuttle}} and {{RexShuttle}} copy operand arrays or lists before knowing
> whether any visited child changes. Read-only traversals of a large literal
> ARRAY therefore allocate replacement operand collections and later return the
> original node.
> The visitors can use copy-on-write behavior and allocate an operand
> collection only after the first child changes. Explicit unconditional-copy
> behavior and immutable-result contracts remain unchanged.
> h3. 5. String literal digest generation creates a temporary SQL string
> {{RexLiteral}} asks {{NlsString}} to build a complete escaped SQL string and
> then copies that string into the destination {{StringBuilder}}. {{NlsString}}
> can provide an overload that writes directly into the caller's builder, while
> the existing string-returning method delegates to the same implementation.
> h2. Combined benchmark
> The changes were measured together because their allocation reductions
> overlap and isolated percentages must not be added.
> || String literals || CALCITE-7782 baseline || All proposed changes ||
> Allocation saved ||
> | 100,000 | 1.522 GB/op | 0.445 GB/op | 1.077 GB/op (70.8%) |
> | 1,000,000 | 15.418 GB/op | 4.556 GB/op | 10.862 GB/op (70.5%) |
> The benchmark uses ordered numeric strings such as {{'1'}}, {{'2'}}, and
> {{'10'}}. The bounded type cache benefits from consecutive literals with the
> same character length. Fixed-width strings should have similar locality
> regardless of value order, while arbitrary strings with frequently
> alternating lengths may show a smaller improvement.
> h2. Proposed delivery
> The implementation is split into five independently reviewable sub-tasks:
> # [CALCITE-7806|https://issues.apache.org/jira/browse/CALCITE-7806] - Avoid
> resolving generated CAST calls during ARRAY validation.
> # [CALCITE-7807|https://issues.apache.org/jira/browse/CALCITE-7807] - Reuse
> unchanged SQL types during least-restrictive inference.
> # [CALCITE-7808|https://issues.apache.org/jira/browse/CALCITE-7808] - Cache
> recently created and decorated SQL types.
> # [CALCITE-7809|https://issues.apache.org/jira/browse/CALCITE-7809] - Avoid
> copying unchanged SQL and Rex operands.
> # [CALCITE-7810|https://issues.apache.org/jira/browse/CALCITE-7810] - Write
> Rex string literal digests directly.
> Each sub-task links its corresponding pull request and can be reviewed and
> tested independently.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
