Fix asymptotic performance issues in live variables analysis. #82
Conversation
Fix possibly-exponential runtime for DFS graph searches. Improve DFA fixpoint algorithm to correctly compute worklist of only changed nodes for each iteration. Added test that takes > 2 minutes to compile without these improvements.
|
Based on #78. I've reformatted the commit message and retargetted the PR to 2.10.x |
|
For the record, the this PR is a lot easier to review with a side-by-side diff, as provided by IOctosplit |
| val preds = asyncStates.filter(_.nextStates.contains(at.state)).toSet | ||
| preds.flatMap(p => lastUsagesOf(field, p, avoid + at)) | ||
| def lastUsagesOf(field: Tree, at: AsyncState): Set[Int] = { | ||
| val avoid = scala.collection.mutable.HashSet[AsyncState]() |
There was a problem hiding this comment.
It seems this optimization (turning the avoid parameter into a local variable, which is a mutable hash set) is independent of the rest of this change set. If so, what's the performance improvement? The code is slightly longer and more imperative, so we might want to have a good reason to do it. What do people think?
There was a problem hiding this comment.
My interpretation of this code is that it's a standard depth-first search through the state graph, just like isPred. If I recall correctly, without this change there is measurably exponential runtime growth when adding an extra level of nesting in the example, which would occur if the state graph has repeatedly-chained 'diamonds', e.g.
1->2
1->3
2->4
3->4
and so forth.
The immutable version doesn't remember that it's traversed the entire subgraph starting from 4, so ends up exploring the exponential number of paths from 1 to the final state.
There was a problem hiding this comment.
That clarifies it- thanks! So, yes, this optimization is definitely essential.
|
LGTM |
Fix asymptotic performance issues in live variables analysis.
Fix possibly-exponential runtime for DFS graph searches.
Improve DFA fixpoint algorithm to correctly compute worklist
of only changed nodes for each iteration.
Added test that takes > 2 minutes to compile without these
improvements.