Skip to content

Java: Fix join-order. #8878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

aschackmull
Copy link
Contributor

This replaces an old-style unbind with a pair of pragmas, and fixes a join-order to put the delta first.

The optimiser would put the visible loop-invariant parts first, and then proceed to early-join the NOT ..#prev to ruin the loop-invariant-ness, which caused each iteration to mostly re-evaluate the same thing. This predicate generally isn't very big, so it wasn't that expensive in wall clock time, but was highlighted by the join-order-badness metric in the nightly dca, so definitely worth fixing.

Before:

Evaluated recursive predicate Validation::validationMethod#d07f4830#bf@03a4awmg in 55ms on iteration 1 (delta size: 48).
Tuple counts for Validation::validationMethod#d07f4830#bf@03a4awmg on iteration 1 running pipeline standard:
         95939    ~5%    {3} r1 = Validation::validationMethod#d07f4830#bf#loop_invariant_prefix AND NOT Validation::validationMethod#d07f4830#bf#prev(Lhs.0, Lhs.1)
         95939    ~0%    {3} r2 = SCAN r1 OUTPUT In.2, In.0, In.1
        158865    ~0%    {3} r3 = JOIN r2 WITH variableBinding_10#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2
         87218    ~0%    {4} r4 = JOIN r3 WITH Expr::MethodAccess::getArgument#dispred#f0820431#fff_201#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Rhs.2
         87207  ~105%    {4} r5 = JOIN r4 WITH Expr::MethodAccess::getMethod#dispred#f0820431#ff ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3
            60    ~0%    {5} r6 = JOIN r5 WITH Validation::validationMethod#d07f4830#bf#prev_delta ON FIRST 1 OUTPUT Lhs.1, Lhs.2, Lhs.3, Lhs.0, Rhs.1
            51    ~0%    {5} r7 = SELECT r6 ON In.2 <= In.4
            48    ~0%    {5} r8 = SELECT r7 ON In.2 >= In.4
            48    ~0%    {2} r9 = SCAN r8 OUTPUT In.0, In.1
                         return r9

After:

Evaluated recursive predicate Validation::validationMethod#d07f4830#ff@2bae8wi7 in 1ms on iteration 1 (delta size: 77).
Tuple counts for Validation::validationMethod#d07f4830#ff@2bae8wi7 on iteration 1 running pipeline standard:
        429  ~4%    {2} r1 = JOIN Validation::validationMethod#d07f4830#ff#prev_delta WITH Expr::MethodAccess::getMethod#dispred#f0820431#ff_10#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.1
        429  ~0%    {1} r2 = JOIN r1 WITH Expr::MethodAccess::getArgument#dispred#f0820431#fff ON FIRST 2 OUTPUT Rhs.2
        204  ~8%    {1} r3 = JOIN r2 WITH variableBinding ON FIRST 1 OUTPUT Rhs.1
        102  ~0%    {2} r4 = JOIN r3 WITH Member::Callable::getParameter#dispred#f0820431#fff_201#join_rhs ON FIRST 1 OUTPUT Rhs.1, Rhs.2
         82  ~0%    {2} r5 = JOIN r4 WITH Member::Method#class#9eba3c33#f ON FIRST 1 OUTPUT Lhs.0, Lhs.1
         77  ~0%    {2} r6 = r5 AND NOT Validation::validationMethod#d07f4830#ff#prev(Lhs.0, Lhs.1)
                    return r6

@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Apr 26, 2022
@aschackmull aschackmull requested a review from a team as a code owner April 26, 2022 11:53
@github-actions github-actions bot added the Java label Apr 26, 2022
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First time I'm seeing the unbind* predicate trick, but this looks plausible to me.

@aschackmull
Copy link
Contributor Author

First time I'm seeing the unbind* predicate trick, but this looks plausible to me.

That was basically what we had to do before we had the only_bind_* pragmas.

@aschackmull aschackmull merged commit 9d2f386 into github:main Apr 28, 2022
@aschackmull aschackmull deleted the java/validationmethod-joinorder branch April 28, 2022 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants