[15] RFR(S): 8246203: Segmentation fault in verification due to stack overflow with -XX:+VerifyIterativeGVN

[Christian Hagedorn]

Hi Tobias

Thank you for your review!

I ran some more testing with -XX:+VerifyIterativeGVN over night and 
compared the iterative solution with the old recursive version. We hit 
some more test timeouts with the new iterative solution because we are 
now really looking at all nodes for a requested depth. There are cases 
where the recursive solution would not do that. For example, if 
verify_depth = 4 and given a node chain 1->2->3->4->5, the recursive DFS 
solution visits nodes 1-4 and then at node 4 when it wants to visit node 
5 it immediately returns because the depth 4 is reached. Node 4 is now 
marked as visited. If, however, there is an additional path 
1->4->5->6->7 to look at later, the recursive DFS solution will just 
stop at node 4 because that node was already visited. The iterative 
solution, on the other hand, processes the nodes in a BFS and will visit 
all nodes up to depth 4, including node 5 and 6. This results in 
spending more time (as seen with more timeouts for more complex 
graphs/tests).

We could now try to simulate the same recursive DFS behavior in an 
iterative approach with a stack. But as we seem to be missing some nodes 
at a requested depth this is probably not what we really want? 
Alternatively, we could go with this BFS solution as it is and decrement 
verify_depth = 4 in the call Node::verify(n, 4) to reduce the time 
spent. Or just leave webrev.01 as it is and treat the additional 
timeouts as expected.

What do you think?

Best regards,
Christian

On 10.06.20 14:30, Tobias Hartmann wrote:
> Hi Christian,
> 
> On 09.06.20 19:32, Christian Hagedorn wrote:
>> http://cr.openjdk.java.net/~chagedorn/8246203/webrev.01/
> 
> Looks good to me.
> 
> Did you run testing with -XX:+VerifyIterativeGVN?
> 
> Best regards,
> Tobias
>