You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@calcite.apache.org by "Dmitry Sysolyatin (Jira)" <ji...@apache.org> on 2022/12/09 16:03:00 UTC

[jira] [Created] (CALCITE-5426) BlockBuilder should not reuse mutable objects

Dmitry Sysolyatin created CALCITE-5426:
------------------------------------------

             Summary: BlockBuilder should not reuse mutable objects
                 Key: CALCITE-5426
                 URL: https://issues.apache.org/jira/browse/CALCITE-5426
             Project: Calcite
          Issue Type: Bug
          Components: linq4j
    Affects Versions: 1.32.0
            Reporter: Dmitry Sysolyatin


Inside BlockBuilder there is an optimization that replaces an expression with a variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One of the problems has been found in CALCITE-5388):

For example:
{code:java}
@Test void testReuseCollectionExpression() throws NoSuchMethodException {
    Method putMethod = HashMap.class.getMethod("put", Object.class, Object.class);
    Method sizeMethod = HashMap.class.getMethod("size");
    
    Expression multiMapParent = b.append("multiMap", Expressions.new_(Types.of(HashMap.class)));
    b.add(Expressions.statement(
        Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), Expressions.box(ONE))));
    
    BlockBuilder nested = new BlockBuilder(true, b);
    Expression multiMapNested = nested.append("multiMap",
        Expressions.new_(Types.of(HashMap.class)));
    nested.add(Expressions.statement(
        Expressions.call(multiMapParent, putMethod, Expressions.box(TWO), Expressions.box(TWO))));
    nested.add(Expressions.call(multiMapNested, sizeMethod));
    
    b.add(nested.toBlock());
    b.append(Expressions.call(multiMapParent, sizeMethod));
    
    // It is wrong output. Map should be reused
    assertEquals(
        "{\n"
            + "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
            + "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
            + "  {\n"
            + "    multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
            + "    return multiMap.size();\n"
            + "  }\n"
            + "  return multiMap.size();\n"
            + "}\n",
        b.toBlock().toString());
  }
{code}

Are there any tests that prove that this optimization significantly improves performance?



--
This message was sent by Atlassian Jira
(v8.20.10#820010)