RFR(XS) 8193577: nsk/jvmti/IterateThroughHeap/filter-tagged fails with Graal in Xcomp mode

[dean.long at oracle.com]

OK your fix looks good.

dl

On 11/28/18 10:57 PM, Igor Veresov wrote:
> It would work right now. But I don’t want us fixing it again when 
> somebody implements effectively final optimization in Graal.
>
> igor
>
>
>
>> On Nov 28, 2018, at 10:02 PM, dean.long at oracle.com 
>> <mailto:dean.long at oracle.com> wrote:
>>
>> I like it better.  But do you really need to use JNI to reset the 
>> values?  If you had
>>
>> static int POISON = 0x1234;  // not final
>>
>> class TaggedClass {
>>       static int field1 = 0xC0DE01 + POISON;
>>
>> in HeapFilter.java, is the compiler smart enough to treat the value 
>> as constant until it changes?
>>
>> dl
>>
>> On 11/28/18 9:26 PM, Igor Veresov wrote:
>>> Alright, how about the following solution: 
>>> http://cr.openjdk.java.net/~iveresov/8193577/webrev.01/
>>>
>>> igor
>>>
>>>
>>>
>>>> On Nov 28, 2018, at 4:59 PM, Igor Veresov <igor.veresov at oracle.com 
>>>> <mailto:igor.veresov at oracle.com>> wrote:
>>>>
>>>> I don’t want to make it Graal specific. I think I’ll just do field 
>>>> assignments in native so that it’s all invisible to the compiler.
>>>>
>>>> igor
>>>>
>>>>
>>>>
>>>>> On Nov 28, 2018, at 3:25 PM, serguei.spitsyn at oracle.com 
>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>
>>>>> On 11/28/18 15:16, dean.long at oracle.com wrote:
>>>>>> It sounds like the test could also fail with C2 if the fields are 
>>>>>> in a virtual object that was eliminated.  I'm OK with your fix, 
>>>>>> but I would feel a little better if we only relaxed the check for 
>>>>>> Graal.  I guess you'd need to use the whitebox api for that.
>>>>>
>>>>> I was thinking about the same.
>>>>> Relaxing this just for Graal sounds good.
>>>>> On the other hand, making the test to know about Graal looks a 
>>>>> little bit strange. :)
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>>
>>>>>> dl
>>>>>>
>>>>>> On 11/28/18 2:28 PM, Igor Veresov wrote:
>>>>>>> Oh, I haven’t understood your idea before pressing reply. Yes, 
>>>>>>> we can match the objects by matching their shape, but it’s also 
>>>>>>> not an exact solution prone to erroneous matches. Especially 
>>>>>>> considering the iteration API does callbacks for the fields out 
>>>>>>> of order - it does primitives, strings, arrays, in that order.
>>>>>>>
>>>>>>> There are also ways to make it fail with Graal that are not 
>>>>>>> related to constant representation. Graal treats allocations as 
>>>>>>> side effect free. So it’s possible to allocate something and 
>>>>>>> then deopt to a point before the allocation and redo the 
>>>>>>> allocation in the interpreter. In this case there are going to 
>>>>>>> be multiple objects on the heap with only one of them being 
>>>>>>> reachable.
>>>>>>>
>>>>>>> igor
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On Nov 28, 2018, at 2:08 PM, Igor Veresov 
>>>>>>>> <igor.veresov at oracle.com <mailto:igor.veresov at oracle.com>> wrote:
>>>>>>>>
>>>>>>>> I don’t think there is a way to identify an untagged object. 
>>>>>>>> There is nothing that is passed in the callback to allow that.
>>>>>>>>
>>>>>>>> igor
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Nov 28, 2018, at 1:32 PM, dean.long at oracle.com 
>>>>>>>>> <mailto:dean.long at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>> I'm guessing Graal creates Constant boxes of the individual 
>>>>>>>>> fields and not of the test objects?  If so, can't we fix the 
>>>>>>>>> matching so that it checks that all fields of an object match? 
>>>>>>>>> I guess that would mean moving the "expected" and "found" 
>>>>>>>>> counts up from fields[] to objects_info[].
>>>>>>>>>
>>>>>>>>> dl
>>>>>>>>>
>>>>>>>>> On 11/28/18 1:13 PM, Igor Veresov wrote:
>>>>>>>>>> When doing heap iteration with JVMTI the way the object 
>>>>>>>>>> identity is tracked is by tagging. This particular test 
>>>>>>>>>> checks if it  can observe an untagged object. Since there is 
>>>>>>>>>> no way to truly track the identity of an untagged object the 
>>>>>>>>>> test validates the identity by checking the value of the 
>>>>>>>>>> fields of such object. When being compiled with Graal there 
>>>>>>>>>> are objects produced (such as Constant boxes) that have field 
>>>>>>>>>> values that are equal to the expected values for the fields 
>>>>>>>>>> in UntaggedClass. During the subsequent heap iteration these 
>>>>>>>>>> objects are reported to the test and are treated as if they 
>>>>>>>>>> were instances of UntaggedClass.
>>>>>>>>>>
>>>>>>>>>> The fix is to relax the test requirement and allow (for the 
>>>>>>>>>> untagged case) the number of the object found during 
>>>>>>>>>> iteration to be more than expected.
>>>>>>>>>>
>>>>>>>>>> Webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~iveresov/8193577/webrev.00/ 
>>>>>>>>>> <http://cr.openjdk.java.net/%7Eiveresov/8193577/webrev.00/>
>>>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8193577
>>>>>>>>>>
>>>>>>>>>> igor
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181129/6c28734c/attachment-0001.html>