Skip to content

Conversation

jbj
Copy link
Contributor

@jbj jbj commented Sep 1, 2018

This is the same change as #123 but for the IR. I still don't know exactly why it's faster, and I invite others to try and replicate my benchmark. I'm using the following query to compare timing and correctness:

import semmle.code.cpp.ir.IR

from int i
where i = any(IRBlock b).getInstructionCount()
select i, strictcount(IRBlock b | i = b.getInstructionCount())

In the QL4E I'm running, the time spent on getInstruction is too low in the "Clause timing report" at the bottom of the log. To see how long the predicate or its replacements actually take, subtract the timestamps before and after in the log output.

@jbj jbj added the C++ label Sep 1, 2018
@jbj jbj added this to the 1.18 milestone Sep 1, 2018
@jbj jbj requested a review from dave-bartolomeo September 1, 2018 18:16
Instead of computing these two things in one predicate, they are
computed in separate predicates and then joined. This splits the
predicate `getInstruction`, which took 81s before, into predicates that
together take 20s on a medium-sized db.
@jbj jbj force-pushed the IRBlock-number-split branch from be617dc to 5541b9f Compare September 5, 2018 06:51
@jbj
Copy link
Contributor Author

jbj commented Sep 5, 2018

Rebased to fix conflicts.

Copy link
Contributor

@dave-bartolomeo dave-bartolomeo 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 you've measured a speedup for this change on a real snapshot, and you've also measured a speedup for the corresponding non-IR change, I'm willing to believe that the speedup is real. The plausible explanations for why it might not be a real speedup (e.g. slightly reduced memory consumption eliminated swapping) were unlikely to hold for both changes.

@jbj
Copy link
Contributor Author

jbj commented Sep 5, 2018

Yes, I tested both this and #123 independently.

@semmle-qlci semmle-qlci merged commit 43e1e62 into github:rc/1.18 Sep 5, 2018
aibaars added a commit that referenced this pull request Oct 14, 2021
AST: RestAssignment and LhsExpr
smowton pushed a commit to smowton/codeql that referenced this pull request Dec 6, 2021
Extract missing functions directly in kotlin package
erik-krogh pushed a commit to erik-krogh/ql that referenced this pull request Dec 15, 2021
Update readme with alerts and actions information
erik-krogh pushed a commit to erik-krogh/ql that referenced this pull request Dec 15, 2021
Update readme with alerts and actions information
MathiasVP added 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