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

[Vladimir Kozlov]

I would keep changes as they are - they provide correct testing. And treat timeouts as expected.

Regards,
Vladimir

On 6/11/20 12:38 AM, Christian Hagedorn wrote:
> 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
>>