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.
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.
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.
The change to make `unresolveElement` a member predicate was helpful for
the optimiser when it dispatched on `this`, but now that it "dispatches"
on `result` it's just an unnecessary pollution of the `ElementBase`
namespace.
This change means that there are no results for `unresolveElement(t)`
where `t` is a "junk type" -- a class definition that is not in the
image of `resolveClass`. These "junk types" still exist as `Element`s,
but they will never be returned by any predicate that goes through
`unresolveElement` to query the db.
We get a small reduction in DIL size and a significant speed
improvement. The DIL for `NewArrayDeleteMismatch.ql` is reduced from
27,630 lines to 27,507 lines, and the total analysis time for the LGTM
suite on jdk8u is reduced from 1158s to 984s.
Raw classes from the database that are incomplete and should be
represented by their complete twin are now allowed to be `Element`s for
performance reasons, but this commit prevents them from being `Type`s.
It was causing confusion in test results and might also cause confusion
in queries.
Also remove the charpred of ElementBase. This gets rid of many redundant
charpred checks. It means that incomplete classes from the db are now
`Element`s, which is maybe noisy but should not be harmful.
Together, these changes give a great reduction in DIL and should help
the optimiser. It brings the DIL of `UncontrolledFormatString.ql` down
from 43,908 lines to 35,400 lines.
We currently erroneously keep mentions of class instantiations, which
can lead to bad performance on template-heavy code bases. We never
want to link those anyway, so we can simply suppress them.
Also exclude templates as their names are not canonical.
The test changes in `isfromtemplateinstantiation/` are the inverses of
what we got in 34c9892f7, which should be a good thing.
