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

Move cached set_is_null/is_null/etc functions from SlotDescriptor to LlvmCodeGen

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: Impala 2.8.0
    • Fix Version/s: Impala 2.8.0
    • Component/s: Backend
    • Labels:
      None

      Description

      This will be required in order to support more flexible sharing of codegen modules between fragments.

        Issue Links

          Activity

          Hide
          tarmstrong Tim Armstrong added a comment -

          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 - 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:
              tarmstrong Tim Armstrong
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development