Skip to content

C++: Fix join order in CommentedOutCode#243

Merged
adityasharad merged 1 commit intogithub:rc/1.18from
jbj:CommentedOutCode-join-fix
Sep 27, 2018
Merged

C++: Fix join order in CommentedOutCode#243
adityasharad merged 1 commit intogithub:rc/1.18from
jbj:CommentedOutCode-join-fix

Conversation

@jbj
Copy link
Copy Markdown
Contributor

@jbj jbj commented Sep 27, 2018

After the recent inlining of unresolveElement, the join order in CommentedOutCode became a problem. The join orderer was tempted to join the two hasLocationInfo calls first because they had one column in common. With this commit, they have no columns in common. It follows from the other predicates in the same file that this column would be the same, so there is no need to assert it in this predicate and risk that the join orderer uses that information.

On Wireshark, the CommentBlock::hasLocationInfo predicate goes from taking 2m2s to taking 180ms. The query produces the same 7,448 alerts.

After the recent inlining of `unresolveElement`, the join order in
`CommentedOutCode` became a problem. The join orderer was tempted to
join the two `hasLocationInfo` calls first because they had one column
in common. With this commit, they have no columns in common. It follows
from the other predicates in the same file that this column would be the
same, so there is no need to assert it in this predicate and risk that
the join orderer uses that information.

On Wireshark, the `CommentBlock::hasLocationInfo` predicate goes from
taking 2m2s to taking 180ms. The query produces the same 7,448 alerts.
@jbj jbj added the C++ label Sep 27, 2018
@jbj jbj added this to the 1.18 milestone Sep 27, 2018
@jbj jbj requested review from ian-semmle and pavgust September 27, 2018 12:14
@adityasharad adityasharad merged commit 1f9da28 into github:rc/1.18 Sep 27, 2018
smowton pushed a commit to smowton/codeql that referenced this pull request Feb 7, 2022
MathiasVP pushed a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants