Skip to content

C++: Fix joins in isModifiableAtImpl #15132

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
merged 1 commit into from
Dec 18, 2023

Conversation

MathiasVP
Copy link
Contributor

Also highlighted by the DCA join order report. This on was actually quite bad 😅.

Before:

[2023-12-18 10:09:33] Evaluated non-recursive predicate SsaInternalsCommon::isModifiableAtImpl/2#2b25ad2f@b02f6ccj in 71404ms (size: 2842043).
Evaluated relational algebra for predicate SsaInternalsCommon::isModifiableAtImpl/2#2b25ad2f@b02f6ccj with tuple counts:
      4208191   ~1%    {2} r1 = JOIN CppType::TCppType#eff8d274 WITH `CppType::CppType.hasType/2#dispred#d3f6ee89` ON FIRST 1 OUTPUT Rhs.1, Lhs.0
      4207753   ~0%    {2} r2 = JOIN r1 WITH `Type::Type.getUnderlyingType/0#dispred#bd141f6a` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
   2914047693   ~0%    {3} r3 = JOIN r2 WITH `Type::Type.getUnderlyingType/0#dispred#bd141f6a_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.0
                  
      1280304   ~0%    {3} r4 = JOIN r3 WITH `SsaInternalsCommon::isIndirectionType/1#6f5ae315` ON FIRST 1 OUTPUT Lhs.2, _, Lhs.1
      1280304   ~0%    {3} r5 = REWRITE r4 WITH Out.1 := 0
      1280304   ~2%    {2} r6 = JOIN r5 WITH `DataFlowUtil::getTypeImpl0/2#26c3c2a8` ON FIRST 2 OUTPUT Rhs.2, Lhs.2
                  
      1280304   ~2%    {2} r7 = JOIN r3 WITH `SsaInternalsCommon::isIndirectionType/1#6f5ae315` ON FIRST 1 OUTPUT Lhs.2, Lhs.1
            0   ~0%    {2} r8 = r7 AND NOT `_project#DataFlowUtil::getTypeImpl0/2#26c3c2a8_10#join_rhs#antijoin_rhs`(FIRST 1)
                  
            0   ~0%    {2} r9 = JOIN r8 WITH Type::UnknownType#cbea2ba5 CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.1
                  
      1280304   ~2%    {2} r10 = r6 UNION r9
                        {2} r11 = r10 AND NOT `_Type::Type.hasSpecifier/1#dispred#d2911bb1#fb_10#join_rhs#antijoin_rhs`(FIRST 1)
      1280304   ~0%    {2} r12 = SCAN r11 OUTPUT In.1, _
      1280304   ~2%    {2} r13 = REWRITE r12 WITH Out.1 := 0
                  
            0   ~0%    {1} r14 = SCAN r8 OUTPUT In.1
            0   ~0%    {2} r15 = JOIN r14 WITH Type::UnknownType#cbea2ba5 CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.0
                  
      1280304   ~2%    {2} r16 = r6 UNION r15
      1279800   ~0%    {2} r17 = JOIN r16 WITH `Type::Type.stripType/0#dispred#e1299c1e` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
      212168   ~0%    {2} r18 = JOIN r17 WITH `project#Class::Class.getAField/0#dispred#19444341` ON FIRST 1 OUTPUT Lhs.1, _
      212168   ~0%    {2} r19 = REWRITE r18 WITH Out.1 := 0
                  
      1492472  ~19%    {2} r20 = r13 UNION r19
                  
      3054211   ~7%    {2} r21 = `SsaInternalsCommon::isModifiableAtImplAtLeast1/2#bae9353a` UNION r20
                        return r21

After:

[2023-12-18 11:59:51] Evaluated non-recursive predicate SsaInternalsCommon::isModifiableAtImpl/2#2b25ad2f@aa81f0vi in 995ms (size: 2842043).
Evaluated relational algebra for predicate SsaInternalsCommon::isModifiableAtImpl/2#2b25ad2f@aa81f0vi with tuple counts:
      4208191   ~1%    {2} r1 = JOIN CppType::TCppType#eff8d274 WITH `CppType::CppType.hasType/2#dispred#d3f6ee89` ON FIRST 1 OUTPUT Rhs.1, Lhs.0
      4207753   ~0%    {2} r2 = JOIN r1 WITH `Type::Type.getUnderlyingType/0#dispred#bd141f6a` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
      1280304   ~2%    {2} r3 = JOIN r2 WITH `SsaInternalsCommon::isUnderlyingIndirectionType/1#14660e47` ON FIRST 1 OUTPUT Lhs.0, Lhs.1
                  
      1280304   ~0%    {3} r4 = JOIN r3 WITH `ResolveClass::isType/1#eabb9878` ON FIRST 1 OUTPUT Lhs.0, _, Lhs.1
      1280304   ~0%    {3} r5 = REWRITE r4 WITH Out.1 := 0
      1280304   ~2%    {2} r6 = JOIN r5 WITH `DataFlowUtil::getTypeImpl0/2#26c3c2a8` ON FIRST 2 OUTPUT Rhs.2, Lhs.2
                  
      1280304   ~2%    {2} r7 = JOIN r3 WITH `ResolveClass::isType/1#eabb9878` ON FIRST 1 OUTPUT Lhs.0, Lhs.1
                        {2} r8 = r7 AND NOT `_project#DataFlowUtil::getTypeImpl0/2#26c3c2a8_10#join_rhs#antijoin_rhs`(FIRST 1)
            0   ~0%    {1} r9 = SCAN r8 OUTPUT In.1
            0   ~0%    {2} r10 = JOIN r9 WITH Type::UnknownType#cbea2ba5 CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.0
                  
      1280304   ~2%    {2} r11 = r6 UNION r10
      1280304   ~2%    {2} r12 = JOIN r11 WITH `ResolveClass::isType/1#eabb9878` ON FIRST 1 OUTPUT Lhs.0, Lhs.1
                  
                        {2} r13 = r12 AND NOT `_Type::Type.hasSpecifier/1#dispred#d2911bb1#fb_10#join_rhs#antijoin_rhs`(FIRST 1)
      1280304   ~0%    {2} r14 = SCAN r13 OUTPUT In.1, _
      1280304   ~2%    {2} r15 = REWRITE r14 WITH Out.1 := 0
                  
      1279800   ~0%    {2} r16 = JOIN r12 WITH `Type::Type.stripType/0#dispred#e1299c1e` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
      212168   ~0%    {2} r17 = JOIN r16 WITH `project#Class::Class.getAField/0#dispred#19444341` ON FIRST 1 OUTPUT Lhs.1, _
      212168   ~0%    {2} r18 = REWRITE r17 WITH Out.1 := 0
                  
      1492472  ~19%    {2} r19 = r15 UNION r18
                  
      3054211   ~7%    {2} r20 = `SsaInternalsCommon::isModifiableAtImplAtLeast1/2#bae9353a` UNION r19
                         return r20

@MathiasVP MathiasVP requested a review from a team as a code owner December 18, 2023 11:04
@github-actions github-actions bot added the C++ label Dec 18, 2023
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Dec 18, 2023
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

LGTM if DCA is happy.

@MathiasVP MathiasVP merged commit d308bb4 into github:main Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ 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