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

[Tobias Hartmann]

+1

Best regards,
Tobias

On 11.06.20 18:27, Vladimir Kozlov wrote:
> 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
>>>