Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5983

Replace for/foreach/map in aggregates by while loops

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: Table API & SQL
    • Labels:
      None

      Description

      Right now there is a mixture of different kinds of loops within aggregate functions. Although performance is not the main goal at the moment, we should focus on performant execution especially in this runtime functions.

      e.g. DataSetTumbleCountWindowAggReduceGroupFunction

      We should replace loops, maps etc. by primitive while loops.

        Issue Links

          Activity

          Hide
          ShaoxuanWang Shaoxuan Wang added a comment -

          I love this proposal to replace scala for loops by while loops in the runtime functions. Thanks Timo Walther.

          Show
          ShaoxuanWang Shaoxuan Wang added a comment - I love this proposal to replace scala for loops by while loops in the runtime functions. Thanks Timo Walther .
          Hide
          ShaoxuanWang Shaoxuan Wang added a comment -

          Can we consider implementing the runtime code and even the built-in aggregate functions in Java

          Show
          ShaoxuanWang Shaoxuan Wang added a comment - Can we consider implementing the runtime code and even the built-in aggregate functions in Java
          Hide
          twalthr Timo Walther added a comment -

          We thought about that multiple times, but the consensus was to let the Table API be implemented entirely in Scala (no mixture of different languages). In a long-term perspective we will code generate the content of the functions anyway, so there will be only Java code executed during runtime and only some Scala code in the open() functions.

          Show
          twalthr Timo Walther added a comment - We thought about that multiple times, but the consensus was to let the Table API be implemented entirely in Scala (no mixture of different languages). In a long-term perspective we will code generate the content of the functions anyway, so there will be only Java code executed during runtime and only some Scala code in the open() functions.
          Hide
          ShaoxuanWang Shaoxuan Wang added a comment -

          ok, the plan sounds good to me. The runtime processing function for aggregate will be codeGened, and it is user's call to use java or scala user defined aggregate functions ---- and if it is written in scala the user is responsible for the performance.

          Show
          ShaoxuanWang Shaoxuan Wang added a comment - ok, the plan sounds good to me. The runtime processing function for aggregate will be codeGened, and it is user's call to use java or scala user defined aggregate functions ---- and if it is written in scala the user is responsible for the performance.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user fhueske opened a pull request:

          https://github.com/apache/flink/pull/3489

          FLINK-5983 [table] Convert FOR into WHILE loops for aggregation functions.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/fhueske/flink tableForToWhile

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3489.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3489


          commit ed336b3970acbfb6b71345745945070ec384442d
          Author: Fabian Hueske <fhueske@apache.org>
          Date: 2017-03-07T21:17:54Z

          FLINK-5983 [table] Convert FOR into WHILE loops for aggregation functions.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user fhueske opened a pull request: https://github.com/apache/flink/pull/3489 FLINK-5983 [table] Convert FOR into WHILE loops for aggregation functions. You can merge this pull request into a Git repository by running: $ git pull https://github.com/fhueske/flink tableForToWhile Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3489.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3489 commit ed336b3970acbfb6b71345745945070ec384442d Author: Fabian Hueske <fhueske@apache.org> Date: 2017-03-07T21:17:54Z FLINK-5983 [table] Convert FOR into WHILE loops for aggregation functions.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user twalthr commented on the issue:

          https://github.com/apache/flink/pull/3489

          Thanks @fhueske. I could not find anything suspicious. I will rebase and merge this...

          Show
          githubbot ASF GitHub Bot added a comment - Github user twalthr commented on the issue: https://github.com/apache/flink/pull/3489 Thanks @fhueske. I could not find anything suspicious. I will rebase and merge this...
          Hide
          twalthr Timo Walther added a comment -

          Fixed in 1.3.0: adbf846f23881b98ab9dc5886a0b066b8aa1ded6

          Show
          twalthr Timo Walther added a comment - Fixed in 1.3.0: adbf846f23881b98ab9dc5886a0b066b8aa1ded6
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3489

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3489

            People

            • Assignee:
              fhueske Fabian Hueske
              Reporter:
              twalthr Timo Walther
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development