In an internal repo we experienced a massive slowdown in compile times after adding a single if block with some instanceof pattern-matching to a function with some existing similarly shaped if blocks. The code looked something like:
if (input.foo() instanceof RedFoo foo
&& foo.bar() instanceof BlueBar bar
&& bar.baz() instanceof GreenBaz baz) {
return baz;
}
After adding this code our full compile times when from about 2 minutes to 12 minutes due to excessive creation of TreeResult values and GC thrashing. Taking a JFR it was clear that SafetyPropagationTransfer was the culprit.
There's a fairly extensive internal thread we're I've recorded my progress while debugging this, so I'm not going to repeat everything here. I do have a minimal repro with the logging that I used to help highlight the problem. This commit also includes a visualization of the CFG of the repro, which I found useful to get a sense for what was happening.
As best I can tell, the issue is that we are not properly returning ConditionalTransferResult for conditional CFG nodes. This causes problems because when performing an analysis, we may have a TransferResult with separate then and else stores, where the then store contains the computed safety result for a pattern-matched variable (the relevant checkerframework code is here). But we end up dropping this computed result due to the leastUpperBound behavior of TransferInputResult.getRegularStore(), which intersects the then and else stores.
This PR changes the behavior of SafetyPropagationTransfer to return ConditionalTransferResult for conditional CFG nodes.
Like most of the code in SafetyPropagationTransfer, I mimicked the behavior of AbstractNullnessPropagationTransfer which does something very similar. I'm not sure why we ever diverged from the AbstractNullnessPropagationTransfer behavior.
After making this change, the test case from repro compiles quickly and without ever triggering an analysis in getCapturedLocalVariableSafety.
svc-autorelease
commented
2 months ago
Problem statement
In an internal repo we experienced a massive slowdown in compile times after adding a single if block with some instanceof pattern-matching to a function with some existing similarly shaped if blocks. The code looked something like:
After adding this code our full compile times when from about 2 minutes to 12 minutes due to excessive creation of
TreeResultvalues and GC thrashing. Taking a JFR it was clear thatSafetyPropagationTransferwas the culprit.There's a fairly extensive internal thread we're I've recorded my progress while debugging this, so I'm not going to repeat everything here. I do have a minimal repro with the logging that I used to help highlight the problem. This commit also includes a visualization of the CFG of the repro, which I found useful to get a sense for what was happening.
The repro is in this commit: https://github.com/palantir/gradle-baseline/commit/4b0286989f894a08765c349fc8c9adec8557954a
Changes in this PR
As best I can tell, the issue is that we are not properly returning
ConditionalTransferResultfor conditional CFG nodes. This causes problems because when performing an analysis, we may have aTransferResultwith separatethenandelsestores, where thethenstore contains the computed safety result for a pattern-matched variable (the relevant checkerframework code is here). But we end up dropping this computed result due to theleastUpperBoundbehavior ofTransferInputResult.getRegularStore(), which intersects thethenandelsestores.This PR changes the behavior of
SafetyPropagationTransferto returnConditionalTransferResultfor conditional CFG nodes.Like most of the code in
SafetyPropagationTransfer, I mimicked the behavior ofAbstractNullnessPropagationTransferwhich does something very similar. I'm not sure why we ever diverged from theAbstractNullnessPropagationTransferbehavior.After making this change, the test case from repro compiles quickly and without ever triggering an analysis in
getCapturedLocalVariableSafety.