Skip to content

Conversation

jbj
Copy link
Contributor

@jbj jbj commented Oct 8, 2018

No description provided.

pavgust and others added 6 commits October 4, 2018 14:25
By pulling out the class `VariableAccessInInitialiser`, we can
avoid some redundant work on pathological databases, improving
performance.
UseInOwnInitialiser: Refactor logic slightly.
The existing implementation of `primitive_basic_block_entry_node` was
"cleverly" computing two properties about `node` with a single
`strictcount`: whether `node` had multiple predecessors and whether any
of those predecessors had more than once successor. This was fast enough
on most snapshots, but on the snapshot of our own code it took 37
seconds to compute `primitive_basic_block_entry_node` and its auxiliary
predicates. This is likely to have affected other large snapshots too.

With this change, the property is computed like in our other languages,
and it brings the run time down to 4 seconds.
This predicate looked like a join of two already-computed predicates,
but it was a bit more complicated because the `*` operator expands into
two cases: the reflexive case and the transitive case. The join order
for the transitive case placed the `PrimitiveBasicBlock` charpred call
_after_ the `member_step+` call, which means that all the tuples of
`member_step+` passed through the pipeline.

This commit changes the implementation by fully writing out the
expansion of `*` into two cases, where the base case is manually
specialised to make sure the join orderer doesn't get tempted into
reusing the same strategy for both cases. This speeds up the predicate
from 2m38s to 1s on a snapshot of our own C/C++ code.
C++: Speed up primitive basic block calculation
@jbj jbj added C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR Mergeback labels Oct 8, 2018
@pavgust pavgust merged commit 2904ebb into github:master Oct 8, 2018
aibaars pushed a commit that referenced this pull request Oct 14, 2021
smowton pushed a commit to smowton/codeql that referenced this pull request Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR Mergeback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants