Skip to content

C++: Fix join order in 'cpp/double-free'. #16089

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 2 commits into from
Apr 2, 2024

Conversation

MathiasVP
Copy link
Contributor

This applies the fix from #3352 to cpp/double-free which suffered from the same issue.

Tuple counts before (on cmake):

Evaluated relational algebra for predicate #select@280ab6i3 with tuple counts:
    11204231  ~0%    {4} r1 = SCAN `Element::ElementBase.toString/0#dispred#6e016f86` OUTPUT In.0, _, In.1, _
    11204231  ~0%    {2}    | REWRITE WITH Tmp.1 := "Memory pointed to by '", Tmp.3 := "' may already have been freed by $@.", Out.1 := (Tmp.1 ++ In.2 ++ Tmp.3) KEEPING 2
        1012  ~3%    {2}    | JOIN WITH `project#FlowAfterFree::isFree/4#cb2bae13#4_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
           5  ~0%    {3}    | JOIN WITH `DataFlowImpl::Impl<DataFlow::DataFlow::GlobalWithState<DoubleFree::DoubleFree::FlowFromFreeConfig>::C>::PathNode.getNode/0#dispred#6f786c66_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.0
           5  ~0%    {4}    | JOIN WITH `DataFlowImpl::Impl<DataFlow::DataFlow::GlobalWithState<DoubleFree::DoubleFree::FlowFromFreeConfig>::C>::flowPath/2#196f9b68_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.0
           5  ~0%    {5}    | JOIN WITH `DataFlowImpl::Impl<DataFlow::DataFlow::GlobalWithState<DoubleFree::DoubleFree::FlowFromFreeConfig>::C>::PathNode.getNode/0#dispred#6f786c66` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3, Lhs.0
           5  ~0%    {5}    | JOIN WITH `project#FlowAfterFree::isFree/4#cb2bae13#6` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3, Lhs.4
           5  ~0%    {6}    | JOIN WITH `Element::ElementBase.toString/0#dispred#6e016f86` ON FIRST 1 OUTPUT Lhs.2, Lhs.4, Lhs.3, Lhs.1, Lhs.0, Rhs.1
                     return r1

After:

Evaluated relational algebra for predicate #select#cpe#1235678@e9734f2j with tuple counts:
    5  ~0%    {2} r1 = SCAN `DataFlowImpl::Impl<DataFlow::DataFlow::GlobalWithState<DoubleFree::DoubleFree::FlowFromFreeConfig>::C>::flowPath/2#196f9b68` OUTPUT In.1, In.0
    5  ~0%    {3}    | JOIN WITH `DataFlowImpl::Impl<DataFlow::DataFlow::GlobalWithState<DoubleFree::DoubleFree::FlowFromFreeConfig>::C>::PathNode.getNode/0#dispred#6f786c66` ON FIRST 1 OUTPUT Lhs.1, Lhs.0, Rhs.1
    5  ~0%    {4}    | JOIN WITH `DataFlowImpl::Impl<DataFlow::DataFlow::GlobalWithState<DoubleFree::DoubleFree::FlowFromFreeConfig>::C>::PathNode.getNode/0#dispred#6f786c66` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.0
    5  ~0%    {4}    | JOIN WITH `project#FlowAfterFree::isFree/4#cb2bae13#6` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3
    5  ~0%    {5}    | JOIN WITH `Element::ElementBase.toString/0#dispred#6e016f86` ON FIRST 1 OUTPUT Lhs.2, Lhs.1, Lhs.3, Lhs.0, Rhs.1
    5  ~0%    {6}    | JOIN WITH `project#FlowAfterFree::isFree/4#cb2bae13#4` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.0, Lhs.2, Lhs.3, Lhs.4
    5  ~0%    {7}    | JOIN WITH `Element::ElementBase.toString/0#dispred#6e016f86` ON FIRST 1 OUTPUT Lhs.2, Lhs.3, Lhs.1, Lhs.0, Rhs.1, Lhs.4, Lhs.5
            return r1

@MathiasVP MathiasVP requested a review from a team as a code owner April 2, 2024 09:03
@github-actions github-actions bot added the C++ label Apr 2, 2024
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Apr 2, 2024
jketema
jketema previously approved these changes Apr 2, 2024
@jketema
Copy link
Contributor

jketema commented Apr 2, 2024

Does this need DCA?

@MathiasVP
Copy link
Contributor Author

Does this need DCA?

Probably not, but I'll start one anyway to be on the safe side. If nothing else, we can get a small dopamine boost from seeing the (probably very small) performance boost 🎉

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