[
https://issues.apache.org/jira/browse/CALCITE-6109?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17785358#comment-17785358
]
asdfgh19 commented on CALCITE-6109:
-----------------------------------
[~julianhyde] Thanks for your suggestion. I changed the description to make the
issue as clear as possible. This is a performance issue that’s hard to ignore.
I can't add a test proving that if this issue isn't fixed it will run out of
memory. Because all new instances it creates are local variables inside the
method.
But I added a test case to the relevant PR to ensure that if the original
statement did not do any optimization, it will not be replaced by the new
instance.
> Avoid extra loops when optimizing statements with ternary expressions
> ---------------------------------------------------------------------
>
> Key: CALCITE-6109
> URL: https://issues.apache.org/jira/browse/CALCITE-6109
> Project: Calcite
> Issue Type: Improvement
> Components: linq4j
> Reporter: asdfgh19
> Assignee: asdfgh19
> Priority: Minor
> Labels: pull-request-available
>
> {code:java}
> // org.apache.calcite.linq4j.tree.BlockBuilder#toBlock
> public BlockStatement toBlock() {
> if (optimizing && removeUnused) {
> // We put an artificial limit of 10 iterations just to prevent an endless
> // loop. Optimize should not loop forever, however it is hard to prove if
> // it always finishes in reasonable time.
> for (int i = 0; i < 10; i++) {
> if (!optimize(createOptimizeShuttle(), true)) {
> break;
> }
> }
> optimize(createFinishingOptimizeShuttle(), false);
> }
> return Expressions.block(statements);
> } {code}
> The above code comes from the org.apache.calcite.linq4j.tree.BlockBuilder
> class in the Calcite linq4j module.
> *1 What is the problem?*
> The problem is that for statements with ternary expressions, the for loop in
> the above code will always execute until the loop is exhausted, although it
> may not have done any optimization.
> *2 How to reproduce this problem？*
> We can reproduce the issue in the following ways.
> # Add some statements with ternary expressions to BlockBuilder through the
> BlockBuilder#add().
> # Call the BlockBuilder#toBlock() method.
> # Observe the for loop in the BlockBuilder#toBlock() method, which is always
> executed 10 times.
> *3 Why does this problem occur？*
> The reason is that when OptimizeShuttle traverses the statement, a new
> instance of TernaryExpression will always be created, regardless of whether
> the optimization is actually performed.
> This makes BlockBuilder mistakenly believe that this optimization is
> effective and start the next optimization.
> {*}4 What impact will this issue have?{*}{*}{*}
> This is a performance issue that’s hard to ignore.
> {code:java}
> return a != 1 ? b : c; {code}
> With a simple line of code like the above, the for loop in the
> org.apache.calcite.linq4j.tree.BlockBuilder#toBlock method will also be
> executed 10 times.
> If there are hundreds or thousands of statements in BlockBuilder#statements,
> this impact cannot be ignored.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
