C++: Fix join-order problem with isBefore#18859
Conversation
Reported here: github#17743 Without this change on the query provided by the user: ``` [2025-02-25 12:42:01] Evaluated non-recursive predicate quickquery::UnrealFunctionAnnotation.annotates/1#dispred#9cd6c269@c668c8tv in 23846ms (size: 20381473). Evaluated relational algebra for predicate quickquery::UnrealFunctionAnnotation.annotates/1#dispred#9cd6c269@c668c8tv with tuple counts: 1 ~0% {0} r1 = CONSTANT()[] 27323 ~0% {2} | JOIN WITH `Location::Location.getEndLine/0#dispred#83af84ae#bf` CARTESIAN PRODUCT OUTPUT Rhs.0, Rhs.1 6162566035 ~0% {4} | JOIN WITH `Location::Location.getStartLine/0#d54f9e6c` CARTESIAN PRODUCT OUTPUT Lhs.0, Lhs.1, Rhs.0, Rhs.1 {4} | REWRITE WITH TEST InOut.1 < InOut.3 3894825644 ~5% {2} | SCAN OUTPUT In.2, In.0 73148692 ~0% {3} | JOIN WITH fun_decls_40#join_rhs ON FIRST 1 OUTPUT Lhs.1, Lhs.0, Rhs.1 73148692 ~0% {4} | JOIN WITH `Location::Location.getFile/0#dispred#d1f8b5d1` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0, Lhs.2 864579 ~0% {2} | JOIN WITH `Location::Location.getFile/0#dispred#d1f8b5d1` ON FIRST 2 OUTPUT Lhs.2, Lhs.3 13010742 ~1% {2} | JOIN WITH macroinvocations_20#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.1 20653781 ~0% {3} | JOIN WITH `Macro::MacroAccess.getOutermostMacroAccess/0#d58b05db_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, _, Lhs.1 20653781 ~4% {3} | REWRITE WITH Out.1 := 1 20381473 ~8% {2} | JOIN WITH macroinvocations_03#join_rhs ON FIRST 2 OUTPUT Lhs.0, Lhs.2 return r1 ``` With this change: ``` [2025-02-25 12:43:10] Evaluated non-recursive predicate quickquery::UnrealFunctionAnnotation.annotates/1#dispred#9cd6c269@11bf8956 in 928ms (size: 20381473). Evaluated relational algebra for predicate quickquery::UnrealFunctionAnnotation.annotates/1#dispred#9cd6c269@11bf8956 with tuple counts: 6873 ~3% {2} r1 = SCAN fun_decls OUTPUT In.4, In.0 6857 ~0% {3} | JOIN WITH `Location::Location.getStartLine/0#d54f9e6c` ON FIRST 1 OUTPUT Lhs.0, Lhs.1, Rhs.1 6857 ~2% {3} | JOIN WITH `Location::Location.getFile/0#dispred#d1f8b5d1` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2 6193961 ~0% {3} | JOIN WITH `Location::Location.getFile/0#dispred#d1f8b5d1_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2 27389714 ~1% {4} | JOIN WITH macroinvocations_20#join_rhs ON FIRST 1 OUTPUT Lhs.0, Lhs.1, Lhs.2, Rhs.1 27389714 ~1% {4} | JOIN WITH locations_default ON FIRST 1 OUTPUT Lhs.1, Lhs.2, Lhs.3, Rhs.4 {4} | REWRITE WITH TEST InOut.3 < InOut.1 13010742 ~1% {2} | SCAN OUTPUT In.2, In.0 20653781 ~0% {3} | JOIN WITH `Macro::MacroAccess.getOutermostMacroAccess/0#d58b05db_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, _, Lhs.1 20653781 ~4% {3} | REWRITE WITH Out.1 := 1 20381473 ~8% {2} | JOIN WITH macroinvocations_03#join_rhs ON FIRST 2 OUTPUT Lhs.0, Lhs.2 return r1 ```
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
geoffw0
left a comment
There was a problem hiding this comment.
Note that we do not use the modified isBefore predicate anywhere in our own code.
That means we get no coverage from DCA. It's difficult to be sure how this will affect performance of any code that uses isBefore apart from the one example from this one user.
I don't have any objections to the change though. It looks cleaner and is potentially easier for the optimizer to work with.
Agreed with all of this. |
Reported here: #17743. Note that we do not use the modified
isBeforepredicate anywhere in our own code.Without this change on the query provided by the user:
With this change: