Skip to content

Commit f3e98c3

Browse files
committed
C++: Fix join order of FlowVar::definedPartiallyAt
This predicate was very slow on kamailio/kamailio: (696s) Tuple counts for FlowVar::FlowVar::definedPartiallyAt_dispred#ff: 703569 ~3% {3} r1 = SCAN FlowVar::FlowVar_internal::TBlockVar#fff AS I OUTPUT I.<1>, I.<0>, I.<2> 7679540588 ~3% {3} r2 = JOIN r1 WITH FlowVar::PartialDefinitions::PartialDefinition::partiallyDefines_dispred#ff_10#join_rhs AS R ON FIRST 1 OUTPUT R.<1>, r1.<1>, r1.<2> 567217 ~2% {2} r3 = JOIN r2 WITH project#FlowVar::PartialDefinitions::PartialDefinition#class#fff#2 AS R ON FIRST 2 OUTPUT r2.<2>, r2.<0> return r3 After this change, the predicate takes no time at all: (22s) Tuple counts for FlowVar::FlowVar::definedPartiallyAt_dispred#ff: 703569 ~3% {3} r1 = SCAN FlowVar::FlowVar_internal::TBlockVar#fff AS I OUTPUT I.<1>, I.<0>, I.<2> 567217 ~2% {2} r2 = JOIN r1 WITH FlowVar::PartialDefinitions::PartialDefinition::partiallyDefinesVariableAt#fff_120#join_rhs AS R ON FIRST 2 OUTPUT r1.<2>, R.<2> return r2 Looking at the code, it turned out that the predicates `partiallyDefines` and `getSubBasicBlockStart` were almost always used together and could therefore be merged into a single predicate to get better join orderings. The predicate `partiallyDefinesThis` was never used.
1 parent 2b720b3 commit f3e98c3

File tree

1 file changed

+13
-9
lines changed
  • cpp/ql/src/semmle/code/cpp/dataflow/internal

1 file changed

+13
-9
lines changed

cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,19 @@ private module PartialDefinitions {
120120
)
121121
}
122122

123-
predicate partiallyDefines(Variable v) { innerDefinedExpr = v.getAnAccess() }
123+
deprecated predicate partiallyDefines(Variable v) { innerDefinedExpr = v.getAnAccess() }
124124

125-
predicate partiallyDefinesThis(ThisExpr e) { innerDefinedExpr = e }
125+
deprecated predicate partiallyDefinesThis(ThisExpr e) { innerDefinedExpr = e }
126126

127127
/**
128-
* Gets the subBasicBlock where this `PartialDefinition` is defined.
128+
* Holds if this `PartialDefinition` defines variable `v` at control-flow
129+
* node `cfn`.
129130
*/
130-
ControlFlowNode getSubBasicBlockStart() { result = node }
131+
pragma[noinline]
132+
predicate partiallyDefinesVariableAt(Variable v, ControlFlowNode cfn) {
133+
innerDefinedExpr = v.getAnAccess() and
134+
cfn = node
135+
}
131136

132137
/**
133138
* Holds if this partial definition may modify `inner` (or what it points
@@ -188,7 +193,7 @@ module FlowVar_internal {
188193
predicate fullySupportedSsaVariable(Variable v) {
189194
v = any(SsaDefinition def).getAVariable() and
190195
// A partially-defined variable is handled using the partial definitions logic.
191-
not any(PartialDefinition p).partiallyDefines(v) and
196+
not any(PartialDefinition p).partiallyDefinesVariableAt(v, _) and
192197
// SSA variables do not exist before their first assignment, but one
193198
// feature of this data flow library is to track where uninitialized data
194199
// ends up.
@@ -232,7 +237,7 @@ module FlowVar_internal {
232237
or
233238
assignmentLikeOperation(sbb, v, _, _)
234239
or
235-
sbb = any(PartialDefinition p | p.partiallyDefines(v)).getSubBasicBlockStart()
240+
exists(PartialDefinition p | p.partiallyDefinesVariableAt(v, sbb))
236241
or
237242
blockVarDefinedByVariable(sbb, v)
238243
)
@@ -363,8 +368,7 @@ module FlowVar_internal {
363368

364369
override predicate definedPartiallyAt(Expr e) {
365370
exists(PartialDefinition p |
366-
p.partiallyDefines(v) and
367-
sbb = p.getSubBasicBlockStart() and
371+
p.partiallyDefinesVariableAt(v, sbb) and
368372
p.definesExpressions(_, e)
369373
)
370374
}
@@ -742,7 +746,7 @@ module FlowVar_internal {
742746
exists(Variable v | not fullySupportedSsaVariable(v) |
743747
assignmentLikeOperation(this, v, _, _)
744748
or
745-
this = any(PartialDefinition p | p.partiallyDefines(v)).getSubBasicBlockStart()
749+
exists(PartialDefinition p | p.partiallyDefinesVariableAt(v, this))
746750
// It is not necessary to cut the basic blocks at `Initializer` nodes
747751
// because the affected variable can have no _other_ value before its
748752
// initializer. It is not necessary to cut basic blocks at procedure

0 commit comments

Comments
 (0)