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

Codegen all aggregate functions, including UDAs

    Details

    • Docs Text:
      Hide
      This fix enables codegen for aggregate functions that previously didn't support codegen. This can lead to dramatic performance improvements (5x in some cases)

      The first part enables codegen for all builtin aggregate functions, except those with CHAR arguments. E.g. min(STRING), sum(TIMESTAMP), group_concat()
      Show
      This fix enables codegen for aggregate functions that previously didn't support codegen. This can lead to dramatic performance improvements (5x in some cases) The first part enables codegen for all builtin aggregate functions, except those with CHAR arguments. E.g. min(STRING), sum(TIMESTAMP), group_concat()
    • Target Version:

      Description

      Currently codegen is disabled for the entire aggregation operator if a single aggregate function can't be codegen'd. We should address this by making it so all aggregate functions can be codegen'd, including UDAs. For UDAs in .so's, the codegen'd function will call into the UDA library. This also affects aggregation operator on timestamp.

      This perf hit can be especially bad for COMPUTE STATS which is heavily CPU bound on the aggregation and because there is no easy way to exclude the TIMESTAMP columns when computing the column stats (i.e., there is no simple workaround).

      Even if the portions involving TIMESTAMP cannot be codegen'd it would still be worthwhile to come up with a workaround where codegen for the other types is still enabled.

      Workaround
      If you are experiencing very slow COMPUTE STATS due to this issue, then you may be able to temporarily ALTER the TIMESTAMP columns to STRING or INT type before running COMPUTE STATS. After the command completed, the columns can be altered back to TIMESTAMP.
      Note the workaround is only apply to text data, not parquet data. parquet require compatibles data type. TIMESTAMP is INT96, it's not compatible with STRING or BIGINT.

        Issue Links

          Activity

          Hide
          tarmstrong Tim Armstrong added a comment -

          IMPALA-1430,IMPALA-4878,IMPALA-4879: codegen native UDAs

          This uses the existing infrastructure for codegening builtin UDAs and
          for codegening calls to UDFs. GetUdf() is refactored to support both
          UDFs and UDAs.

          IR UDAs are still not allowed by the frontend. It's unclear if we want
          to enable them going forward because of the difficulties in testing and
          supporting IR UDFs/UDAs.

          This also fixes some bugs with the Get*Type() methods of
          FunctionContext. GetArgType() and related methods now always return the
          logical input types of the aggregate function. Getting the tests to pass
          required fixing IMPALA-4878 because they called GetIntermediateType().

          Testing:
          test_udfs.py tests UDAs with codegen enabled and disabled.

          Added some asserts to test UDAs to check that the correct types are
          passed in via the FunctionContext.

          Change-Id: Id1708eaa96eb76fb9bec5eeabf209f81c88eec2f
          Reviewed-on: http://gerrit.cloudera.org:8080/5161
          Reviewed-by: Dan Hecht <dhecht@cloudera.com>
          Tested-by: Impala Public Jenkins

          Show
          tarmstrong Tim Armstrong added a comment - IMPALA-1430 , IMPALA-4878 , IMPALA-4879 : codegen native UDAs This uses the existing infrastructure for codegening builtin UDAs and for codegening calls to UDFs. GetUdf() is refactored to support both UDFs and UDAs. IR UDAs are still not allowed by the frontend. It's unclear if we want to enable them going forward because of the difficulties in testing and supporting IR UDFs/UDAs. This also fixes some bugs with the Get*Type() methods of FunctionContext. GetArgType() and related methods now always return the logical input types of the aggregate function. Getting the tests to pass required fixing IMPALA-4878 because they called GetIntermediateType(). Testing: test_udfs.py tests UDAs with codegen enabled and disabled. Added some asserts to test UDAs to check that the correct types are passed in via the FunctionContext. Change-Id: Id1708eaa96eb76fb9bec5eeabf209f81c88eec2f Reviewed-on: http://gerrit.cloudera.org:8080/5161 Reviewed-by: Dan Hecht <dhecht@cloudera.com> Tested-by: Impala Public Jenkins
          Hide
          tarmstrong Tim Armstrong added a comment -

          This commit fixes this for builtin aggregate functions (not external UDAs)

          IMPALA-1430,IMPALA-4108: codegen all builtin aggregate functions

          This change enables codegen for all builtin aggregate functions,
          e.g. timestamp functions and group_concat.

          There are several parts to the change:

          • Adding support for generic UDAs. Previous the codegen code did not
            handle multiple input arguments or NULL return values.
          • Defaulting to using the UDA interface when there is not a special
            codegen path (we have implementations of all builtin aggregate
            functions for the interpreted path).
          • Remove all the logic to disable codegen for the special cases that now
            are supported.

          Also fix the generation of code to get/set NULL bits since I needed
          to add functionality there anyway.

          Testing:
          Add tests that check that codegen was enabled for builtin aggregate
          functions. Also fix some gaps in the preexisting tests.

          Also add tests for UDAs that check input/output nulls are handled
          correctly, in anticipation of enabling codegen for arbitrary UDAs.

          Perf:
          Ran local TPC-H and targeted perf. Spent a lot of time on TPC-H Q1,
          since my original approach regressed it ~5%. In the end the problem was
          to do with the ordering of loads/stores to the slot and null bit in the
          generated code: the previous version of the code exploited some
          properties of the particular aggregate function. I ended up replicating
          this behaviour to avoid regressing perf.

          Change-Id: Id9dc21d1d676505d3617e1e4f37557397c4fb260

          Show
          tarmstrong Tim Armstrong added a comment - This commit fixes this for builtin aggregate functions (not external UDAs) IMPALA-1430 , IMPALA-4108 : codegen all builtin aggregate functions This change enables codegen for all builtin aggregate functions, e.g. timestamp functions and group_concat. There are several parts to the change: Adding support for generic UDAs. Previous the codegen code did not handle multiple input arguments or NULL return values. Defaulting to using the UDA interface when there is not a special codegen path (we have implementations of all builtin aggregate functions for the interpreted path). Remove all the logic to disable codegen for the special cases that now are supported. Also fix the generation of code to get/set NULL bits since I needed to add functionality there anyway. Testing: Add tests that check that codegen was enabled for builtin aggregate functions. Also fix some gaps in the preexisting tests. Also add tests for UDAs that check input/output nulls are handled correctly, in anticipation of enabling codegen for arbitrary UDAs. Perf: Ran local TPC-H and targeted perf. Spent a lot of time on TPC-H Q1, since my original approach regressed it ~5%. In the end the problem was to do with the ordering of loads/stores to the slot and null bit in the generated code: the previous version of the code exploited some properties of the particular aggregate function. I ended up replicating this behaviour to avoid regressing perf. Change-Id: Id9dc21d1d676505d3617e1e4f37557397c4fb260

            People

            • Assignee:
              tarmstrong Tim Armstrong
              Reporter:
              skye Skye Wanderman-Milne
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development