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/
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] 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
igor