Skip to content

Conversation

@MathiasVP
Copy link
Contributor

Before (when running cpp/inconsistent-null-check on zeek/spicy):

Evaluated non-recursive predicate Element::Element.getFile/0#dispred#536cb5f3#bb@f6f5329i in 182326ms (size: 50437). Evaluated relational algebra for predicate Element::Element.getFile/0#dispred#536cb5f3#bb@f6f5329i with tuple counts:
           2029351   ~0%    {2} r1 = SCAN `Expr::Expr.getLocation/0#dispred#0a3d90c6` OUTPUT In.1, In.0
           2029351   ~0%    {2}    | JOIN WITH `Location::Location.getStartLine/0#d54f9e6c` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
           1168789   ~0%    {2}    | JOIN WITH `InconsistentCheckReturnNull::assertInvocation/2#b2a4c9e3_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
        5533128288   ~0%    {3}    | JOIN WITH `Location::Location.getContainer/0#9edabfb6_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
             50413   ~0%    {2}    | JOIN WITH `Element::Element.getLocation/0#dispred#6c3f5b09#bf` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

              3043   ~0%    {2} r2 = JOIN `project#InconsistentCheckReturnNull::relevantFunctionCall/2#d18cd566` WITH `Expr::Expr.getLocation/0#dispred#0a3d90c6` ON FIRST 1 OUTPUT Rhs.1, Lhs.0

              3043   ~0%    {2} r3 = JOIN r2 WITH locations_default ON FIRST 1 OUTPUT Rhs.4, Lhs.1
              1945   ~3%    {2}    | JOIN WITH `InconsistentCheckReturnNull::assertInvocation/2#b2a4c9e3_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
           9106248   ~2%    {3}    | JOIN WITH `Location::Location.getContainer/0#9edabfb6_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
                 0   ~0%    {2}    | JOIN WITH `Element::Element.getLocation/0#dispred#6c3f5b09#bf` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

              3043   ~0%    {3} r4 = JOIN r2 WITH locations_default ON FIRST 1 OUTPUT _, Lhs.1, Rhs.4
              3043   ~0%    {2}    | REWRITE WITH Tmp.0 := 1, Out.0 := (In.2 + Tmp.0) KEEPING 2
              2013   ~0%    {2}    | JOIN WITH `InconsistentCheckReturnNull::assertInvocation/2#b2a4c9e3_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
           9621327   ~0%    {3}    | JOIN WITH `Location::Location.getContainer/0#9edabfb6_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
                24   ~3%    {2}    | JOIN WITH `Element::Element.getLocation/0#dispred#6c3f5b09#bf` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

             50437   ~0%    {2} r5 = r1 UNION r3 UNION r4
                            return r5

After:

[2025-10-02 17:38:06] Evaluated non-recursive predicate Element::Element.getFile/0#2b8c8740@27cef30e in 154ms (size: 8896041).
Evaluated relational algebra for predicate Element::Element.getFile/0#2b8c8740@27cef30e with tuple counts:
        8897861  ~0%    {2} r1 = SCAN `Element::Element.getLocation/0#dispred#6c3f5b09` OUTPUT In.1, In.0
        8897861  ~2%    {2}    | JOIN WITH `Location::Location.getContainer/0#9edabfb6` ON FIRST 1 OUTPUT Lhs.1, Rhs.1
                        return r1

…heckReturnNull.qll:

Evaluated non-recursive predicate Element::Element.getFile/0#dispred#536cb5f3#bb@f6f5329i in 182326ms (size: 50437).
Evaluated relational algebra for predicate Element::Element.getFile/0#dispred#536cb5f3#bb@f6f5329i with tuple counts:
           2029351   ~0%    {2} r1 = SCAN `Expr::Expr.getLocation/0#dispred#0a3d90c6` OUTPUT In.1, In.0
           2029351   ~0%    {2}    | JOIN WITH `Location::Location.getStartLine/0#d54f9e6c` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
           1168789   ~0%    {2}    | JOIN WITH `InconsistentCheckReturnNull::assertInvocation/2#b2a4c9e3_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
        5533128288   ~0%    {3}    | JOIN WITH `Location::Location.getContainer/0#9edabfb6_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
             50413   ~0%    {2}    | JOIN WITH `Element::Element.getLocation/0#dispred#6c3f5b09#bf` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

              3043   ~0%    {2} r2 = JOIN `project#InconsistentCheckReturnNull::relevantFunctionCall/2#d18cd566` WITH `Expr::Expr.getLocation/0#dispred#0a3d90c6` ON FIRST 1 OUTPUT Rhs.1, Lhs.0

              3043   ~0%    {2} r3 = JOIN r2 WITH locations_default ON FIRST 1 OUTPUT Rhs.4, Lhs.1
              1945   ~3%    {2}    | JOIN WITH `InconsistentCheckReturnNull::assertInvocation/2#b2a4c9e3_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
           9106248   ~2%    {3}    | JOIN WITH `Location::Location.getContainer/0#9edabfb6_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
                 0   ~0%    {2}    | JOIN WITH `Element::Element.getLocation/0#dispred#6c3f5b09#bf` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

              3043   ~0%    {3} r4 = JOIN r2 WITH locations_default ON FIRST 1 OUTPUT _, Lhs.1, Rhs.4
              3043   ~0%    {2}    | REWRITE WITH Tmp.0 := 1, Out.0 := (In.2 + Tmp.0) KEEPING 2
              2013   ~0%    {2}    | JOIN WITH `InconsistentCheckReturnNull::assertInvocation/2#b2a4c9e3_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
           9621327   ~0%    {3}    | JOIN WITH `Location::Location.getContainer/0#9edabfb6_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
                24   ~3%    {2}    | JOIN WITH `Element::Element.getLocation/0#dispred#6c3f5b09#bf` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

             50437   ~0%    {2} r5 = r1 UNION r3 UNION r4
                            return r5
@MathiasVP MathiasVP requested a review from a team as a code owner October 2, 2025 16:41
Copilot AI review requested due to automatic review settings October 2, 2025 16:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a pragma[nomagic] annotation to the Element.getFile() method in the C++ library to improve query performance by preventing the magic set optimization from being applied to this predicate.

  • Adds pragma[nomagic] annotation to Element.getFile() method
  • Improves query evaluation performance by orders of magnitude (from 182326ms to 154ms)
  • Simplifies the relational algebra evaluation plan

@github-actions github-actions bot added the C++ label Oct 2, 2025
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Oct 2, 2025
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Obviously the change is behaviour preserving, and your stats look good. I think we just need to wait for the DCA run to finish...

@MathiasVP
Copy link
Contributor Author

MathiasVP commented Oct 2, 2025

Obviously the change is behaviour preserving, and your stats look good. I think we just need to wait for the DCA run to finish...

That's great, thanks! Do you mind approving it so that I can merge it later today if DCA looks fine?

@jketema jketema merged commit 7807804 into github:main Oct 2, 2025
16 of 17 checks passed
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.

3 participants