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

Propagate the argument 'type' for RawValue::Compare()

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: Impala 2.9.0
    • Fix Version/s: Impala 2.10.0
    • Component/s: Backend
    • Labels:
    • Epic Color:
      ghx-label-1

      Description

      CodegenAnyVal::Compare() generates code which calls the cross
      compiled version of RawValue::Compare() without propagating
      the type information into RawValue::Compare(). As a result,
      the generated code of RawValue::Compare() is not any more
      efficient than the interpreted version as we still have
      the big switch statement in it.

        Activity

        Hide
        kwho Michael Ho added a comment -

        IMPALA-5479: Propagate the argument type in RawValue::Compare()
        CodegenAnyVal::Compare() generates code which calls the cross
        compiled version of RawValue::Compare() without propagating
        the type information into RawValue::Compare(). As a result,
        the generated code of RawValue::Compare() is not any more
        efficient than the interpreted version as we still have
        the big switch statement in it.

        This change creates a global constant for the argument 'type'
        passed to RawValue::Compare(). By inlining the call to
        RawValue::Compare(), LLVM was able to constant propagate
        the type and eliminate the dead code for non-target types.

        With this change, a query with top-n improves by 12% on average:

        select l_orderkey, l_partkey, l_suppkey
        from tpch50_parquet.lineitem
        order by l_orderkey, l_partkey, l_suppkey
        limit 10000;

        4.49s -> 3.95s

        This change also adds the ALWAYS_INLINE attribute to
        RuntimeFilter::Eval() as it's needed to propagate the type
        after a recent change to not put ALWAYS_INLINE attribute
        on all cross-compiled functions.

        Change-Id: I10b5b284e3da03024476a9620a12d6e7fbf08b3c
        Reviewed-on: http://gerrit.cloudera.org:8080/7140
        Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
        Tested-by: Impala Public Jenkins

        Show
        kwho Michael Ho added a comment - IMPALA-5479 : Propagate the argument type in RawValue::Compare() CodegenAnyVal::Compare() generates code which calls the cross compiled version of RawValue::Compare() without propagating the type information into RawValue::Compare(). As a result, the generated code of RawValue::Compare() is not any more efficient than the interpreted version as we still have the big switch statement in it. This change creates a global constant for the argument 'type' passed to RawValue::Compare(). By inlining the call to RawValue::Compare(), LLVM was able to constant propagate the type and eliminate the dead code for non-target types. With this change, a query with top-n improves by 12% on average: select l_orderkey, l_partkey, l_suppkey from tpch50_parquet.lineitem order by l_orderkey, l_partkey, l_suppkey limit 10000; 4.49s -> 3.95s This change also adds the ALWAYS_INLINE attribute to RuntimeFilter::Eval() as it's needed to propagate the type after a recent change to not put ALWAYS_INLINE attribute on all cross-compiled functions. Change-Id: I10b5b284e3da03024476a9620a12d6e7fbf08b3c Reviewed-on: http://gerrit.cloudera.org:8080/7140 Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com> Tested-by: Impala Public Jenkins
        Hide
        laszlog Laszlo Gaal added a comment -

        Michael Ho, looking at the code I'm afraid this patch is not in 2.9.0. Do you mind if I update the FixVersion to 2.10.0? Thanks a lot!

        Show
        laszlog Laszlo Gaal added a comment - Michael Ho , looking at the code I'm afraid this patch is not in 2.9.0. Do you mind if I update the FixVersion to 2.10.0? Thanks a lot!
        Hide
        kwho Michael Ho added a comment -

        Laszlo Gaal, thanks for noticing it. It's fixed now.

        Show
        kwho Michael Ho added a comment - Laszlo Gaal , thanks for noticing it. It's fixed now.

          People

          • Assignee:
            kwho Michael Ho
            Reporter:
            kwho Michael Ho
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development