Thanks Dean and Serguei! igor
> On Nov 29, 2018, at 8:00 AM, [email protected] wrote: > > Hi Igor, > > +1 > > Thanks, > Serguei > > > On 11/29/18 07:42, [email protected] <mailto:[email protected]> wrote: >> 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, [email protected] >>>> <mailto:[email protected]> 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/ >>>>> <http://cr.openjdk.java.net/%7Eiveresov/8193577/webrev.01/> >>>>> >>>>> igor >>>>> >>>>> >>>>> >>>>>> On Nov 28, 2018, at 4:59 PM, Igor Veresov <[email protected] >>>>>> <mailto:[email protected]>> 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, [email protected] >>>>>>> <mailto:[email protected]> wrote: >>>>>>> >>>>>>> On 11/28/18 15:16, [email protected] <mailto:[email protected]> >>>>>>> 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 <[email protected] >>>>>>>>>> <mailto:[email protected]>> 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, [email protected] >>>>>>>>>>> <mailto:[email protected]> 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 >>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8193577> >>>>>>>>>>>> >>>>>>>>>>>> igor >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
