Java: Adjust caching of BasicBlocks, BaseSSA, and CompileTimeConstants #19093
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -24,7 +40,10 @@ | |||
|
|
|||
| /** Gets an immediate successor of this basic block. */ | |||
| cached | |||
| BasicBlock getABBSuccessor() { result = this.getLastNode().getASuccessor() } | |||
| BasicBlock getABBSuccessor() { | |||
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
aschackmull
added
the
no-change-note-required
aschackmull
changed the title
|
Dca looks like a decent performance improvement. |
Also a decent DIL size improvement. |
| private predicate localFlowPlusInitializers(DataFlow::Node pred, DataFlow::Node succ) { | ||
| exists(Variable v | v.isFinal() and pred.asExpr() = v.getInitializer() | | ||
| DataFlow::localFlow(DataFlow::exprNode(v.getAnAccess()), succ) | ||
| private predicate localFlowStepPlusInitializers(DataFlow::Node pred, DataFlow::Node succ) { |
There was a problem hiding this comment.
Is this rewrite semantics preserving? It looks like before it would calculate flow as
pred --initializer--> access --local--*> succ
and now it is
pred --initializer/local--*> succ
There was a problem hiding this comment.
Correct, it's not 100% semantics preserving, but I think the tweaked version makes more sense: If we're doing local flow plus initialization of final fields, then it seems like a mistake to rule out local flow prior to the initialization, like e.g. a cast or something like that.
This PR makes 3 changes to caching in the Java QL libs (separated cleanly in individual commits):
All 3 of these changes were motivated by statically inspecting DIL overlap between stages. These cases stood out as potentially being able to cause recomputation.
The changes caused a bit of performance fallout, which was fixed in an additional commit.