Uploaded image for project: 'IMPALA'
  1. IMPALA
  2. IMPALA-2379

Constant argument for UDAF is not accessible in Init function.

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: Impala 2.2
    • Fix Version/s: Impala 2.8.0
    • Component/s: Frontend
    • Labels:

      Description

      A user reported on the user list that they were unable to access a constant argument from the Init function of a UDAF. It would be convenient in many cases to support this.

      I'm adding a parameter to my version of the appx_median function to set the number of samples we want to store. My plan was to require that the number of samples parameter be a constant so I can allocate the memory for the samples in the Init function but I can't seem to access it in the Init function.
      
        if (! ctx->IsArgConstant(1)) {
          ctx->SetError("The number of samples must be a constant");
          return;
      
        }
      
      If I include that check in my Init function, it always returns an error and I get back a NULL when I try to access the constant arg:
      
      int max_samples = reinterpret_cast<IntVal*>(ctx->GetConstantArg(1))->val;
      

        Activity

        Hide
        tarmstrong Tim Armstrong added a comment -

        I was talking about this with Chris a while ago when he was investigating and it looked like this is actually a bug to do with when the Init() function is called by the PAGG Node. I.e. it's not just a missing feature.

        Show
        tarmstrong Tim Armstrong added a comment - I was talking about this with Chris a while ago when he was investigating and it looked like this is actually a bug to do with when the Init() function is called by the PAGG Node. I.e. it's not just a missing feature.
        Hide
        tarmstrong Tim Armstrong added a comment -

        IMPALA-4302,IMPALA-2379: constant expr arg fixes

        This patch fixes two issues around handling of constant expr args.
        The patches are combined because they touch some of the same code
        and depend on some of the same memory management cleanup.

        First, it fixes IMPALA-2379, where constant expr args were not visible
        to UDAFs. The issue is that the input exprs need to be opened before
        calling the UDAF Init() function.

        Second, it avoids overhead from repeated evaluation of constant
        arguments for ScalarFnCall expressions on both the codegen'd and
        interpreted paths. A common example is an IN predicate with a
        long list of constant values.

        The interpreted path was inefficient because it always evaluated all
        children expressions. Instead in this patch constant args are
        evaluated once and cached. The memory management of the AnyVal*
        objects was somewhat nebulous - adjusted it so that they're allocated
        from ExprContext::mem_pool_, which has the correct lifetime.

        The codegen'd path was inefficient only with varargs - with fixed
        arguments the LLVM optimiser is able to infer after inlining that
        the expressions are constant and remove all evaluation. However,
        for varargs it stores the vararg values into a heap-allocated buffer.
        The LLVM optimiser is unable to remove these stores because they
        have a side-effect that is visible to code outside the function.

        The codegen'd path is improved by evaluating varargs into an automatic
        buffer that can be optimised out. We also make a small related change
        to bake the string constants into the codegen'd code.

        Testing:
        Ran exhaustive build.

        Added regression test for IMPALA-2379 and MemPool test for aligned
        allocation. Added a test for in predicates with constant strings.

        Perf:
        Added a targeted query that demonstrates the improvement. Also manually
        validated the non-codegend perf. Also ran TPC-H and targeted perf
        queries locally - didn't see any significant changes.

        ----------------------------------------------------------------------------------------------------------------------------------------+

        Workload Query File Format Avg(s) Base Avg(s) Delta(Avg) StdDev(%) Base StdDev(%) Num Clients Iters

        ----------------------------------------------------------------------------------------------------------------------------------------+

        TARGETED-PERF(_20) primitive_filter_in_predicate parquet / none / none 1.19 9.82 I -87.85% 3.82% 0.71% 1 10

        ----------------------------------------------------------------------------------------------------------------------------------------+

        (I) Improvement: TARGETED-PERF(_20) primitive_filter_in_predicate [parquet / none / none] (9.82s -> 1.19s [-87.85%])
        ----------------------------------------------------------------------------------------------------------+

        Operator % of Query Avg Base Avg Delta(Avg) StdDev(%) Max Base Max Delta(Max) #Hosts #Rows Est #Rows

        ----------------------------------------------------------------------------------------------------------+

        01:AGGREGATE 14.39% 155.88ms 214.61ms -27.37% 2.68% 163.38ms 227.53ms -28.19% 1 1 1
        00:SCAN HDFS 85.60% 927.46ms 9.43s -90.16% 4.49% 1.01s 9.50s -89.42% 1 13.77K 14.05K

        ----------------------------------------------------------------------------------------------------------+

        Change-Id: I45c3ed8c9d7a61e94a9b9d6c316e8a53d9ff6c24
        Reviewed-on: http://gerrit.cloudera.org:8080/4838
        Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
        Tested-by: Internal Jenkins

        Show
        tarmstrong Tim Armstrong added a comment - IMPALA-4302 , IMPALA-2379 : constant expr arg fixes This patch fixes two issues around handling of constant expr args. The patches are combined because they touch some of the same code and depend on some of the same memory management cleanup. First, it fixes IMPALA-2379 , where constant expr args were not visible to UDAFs. The issue is that the input exprs need to be opened before calling the UDAF Init() function. Second, it avoids overhead from repeated evaluation of constant arguments for ScalarFnCall expressions on both the codegen'd and interpreted paths. A common example is an IN predicate with a long list of constant values. The interpreted path was inefficient because it always evaluated all children expressions. Instead in this patch constant args are evaluated once and cached. The memory management of the AnyVal* objects was somewhat nebulous - adjusted it so that they're allocated from ExprContext::mem_pool_, which has the correct lifetime. The codegen'd path was inefficient only with varargs - with fixed arguments the LLVM optimiser is able to infer after inlining that the expressions are constant and remove all evaluation. However, for varargs it stores the vararg values into a heap-allocated buffer. The LLVM optimiser is unable to remove these stores because they have a side-effect that is visible to code outside the function. The codegen'd path is improved by evaluating varargs into an automatic buffer that can be optimised out. We also make a small related change to bake the string constants into the codegen'd code. Testing: Ran exhaustive build. Added regression test for IMPALA-2379 and MemPool test for aligned allocation. Added a test for in predicates with constant strings. Perf: Added a targeted query that demonstrates the improvement. Also manually validated the non-codegend perf. Also ran TPC-H and targeted perf queries locally - didn't see any significant changes. ------------------- ----------------------------- --------------------- ------ ----------- ---------- --------- -------------- ----------- ------+ Workload Query File Format Avg(s) Base Avg(s) Delta(Avg) StdDev(%) Base StdDev(%) Num Clients Iters ------------------- ----------------------------- --------------------- ------ ----------- ---------- --------- -------------- ----------- ------+ TARGETED-PERF(_20) primitive_filter_in_predicate parquet / none / none 1.19 9.82 I -87.85% 3.82% 0.71% 1 10 ------------------- ----------------------------- --------------------- ------ ----------- ---------- --------- -------------- ----------- ------+ (I) Improvement: TARGETED-PERF(_20) primitive_filter_in_predicate [parquet / none / none] (9.82s -> 1.19s [-87.85%] ) ------------- ---------- -------- -------- ---------- --------- -------- -------- ---------- ------ ------ ----------+ Operator % of Query Avg Base Avg Delta(Avg) StdDev(%) Max Base Max Delta(Max) #Hosts #Rows Est #Rows ------------- ---------- -------- -------- ---------- --------- -------- -------- ---------- ------ ------ ----------+ 01:AGGREGATE 14.39% 155.88ms 214.61ms -27.37% 2.68% 163.38ms 227.53ms -28.19% 1 1 1 00:SCAN HDFS 85.60% 927.46ms 9.43s -90.16% 4.49% 1.01s 9.50s -89.42% 1 13.77K 14.05K ------------- ---------- -------- -------- ---------- --------- -------- -------- ---------- ------ ------ ----------+ Change-Id: I45c3ed8c9d7a61e94a9b9d6c316e8a53d9ff6c24 Reviewed-on: http://gerrit.cloudera.org:8080/4838 Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com> Tested-by: Internal Jenkins

          People

          • Assignee:
            cchanning Chris Channing
            Reporter:
            tarmstrong Tim Armstrong
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development