Discussion:
RFR(XS) 8193577: nsk/jvmti/IterateThroughHeap/filter-tagged fails with Graal in Xcomp mode
(too old to reply)
Igor Veresov
2018-11-28 21:13:09 UTC
Permalink
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/
JBS: https://bugs.openjdk.java.net/browse/JDK-8193577

igor
s***@oracle.com
2018-11-28 21:27:15 UTC
Permalink
Hi Igor,

It looks like a hack but Okay with me.

Thanks,
Serguei
Post by Igor Veresov
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/
JBS: https://bugs.openjdk.java.net/browse/JDK-8193577
igor
Igor Veresov
2018-11-28 22:12:28 UTC
Permalink
I don’t see a better way, really. If there is a less hacky way that is allowed by the iteration API, I’m definitely open to suggestions.

igor
Post by s***@oracle.com
Hi Igor,
It looks like a hack but Okay with me.
Thanks,
Serguei
Post by Igor Veresov
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/
JBS: https://bugs.openjdk.java.net/browse/JDK-8193577
igor
d***@oracle.com
2018-11-28 21:32:58 UTC
Permalink
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
Post by Igor Veresov
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/
JBS: https://bugs.openjdk.java.net/browse/JDK-8193577
igor
Igor Veresov
2018-11-28 22:08:58 UTC
Permalink
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
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
Post by Igor Veresov
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/
JBS: https://bugs.openjdk.java.net/browse/JDK-8193577
igor
Igor Veresov
2018-11-28 22:28:25 UTC
Permalink
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
Post by Igor Veresov
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
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
Post by Igor Veresov
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/~iveresov/8193577/webrev.00/>
JBS: https://bugs.openjdk.java.net/browse/JDK-8193577 <https://bugs.openjdk.java.net/browse/JDK-8193577>
igor
d***@oracle.com
2018-11-28 23:16:30 UTC
Permalink
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.

dl
Post by Igor Veresov
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
Post by Igor Veresov
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
Post by d***@oracle.com
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
Post by Igor Veresov
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/
JBS: https://bugs.openjdk.java.net/browse/JDK-8193577
igor
s***@oracle.com
2018-11-28 23:25:34 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 11/28/18 15:16, <a class="moz-txt-link-abbreviated" href="mailto:***@oracle.com">***@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:e97be0a0-89ca-1a1e-3c4f-***@oracle.com"> 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.<br>
</blockquote>
<br>
I was thinking about the same.<br>
Relaxing this just for Graal sounds good.<br>
On the other hand, making the test to know about Graal looks a
little bit strange. :)<br>
<br>
Thanks,<br>
Serguei <br>
<br>
<blockquote type="cite"
cite="mid:e97be0a0-89ca-1a1e-3c4f-***@oracle.com"> <br>
dl<br>
<br>
<div class="moz-cite-prefix">On 11/28/18 2:28 PM, Igor Veresov
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:923D4C45-2684-41F6-9CD7-***@oracle.com">
<div class="">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.</div>
<div class=""><br class="">
</div>
<div class="">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.</div>
<div class=""><br class="">
</div>
<div class=""> <span class="Apple-style-span">igor<br class="">
<br class="">
<br class="">
</span> </div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">On Nov 28, 2018, at 2:08 PM, Igor Veresov &lt;<a
href="mailto:***@oracle.com" class=""
moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<div class="">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.</div>
<br class="">
<div class=""> <span class="Apple-style-span">igor<br
class="">
<br class="">
</span> </div>
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Nov 28, 2018, at 1:32 PM, <a
href="mailto:***@oracle.com" class=""
moz-do-not-send="true">***@oracle.com</a>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">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[].<br class="">
<br class="">
dl<br class="">
<br class="">
On 11/28/18 1:13 PM, Igor Veresov wrote:<br
class="">
<blockquote type="cite" class="">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.<br
class="">
<br class="">
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.<br class="">
<br class="">
Webrev: <a
href="http://cr.openjdk.java.net/%7Eiveresov/8193577/webrev.00/"
class="" moz-do-not-send="true">http://cr.openjdk.java.net/~iveresov/8193577/webrev.00/</a><br
class="">
JBS: <a
href="https://bugs.openjdk.java.net/browse/JDK-8193577"
class="" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8193577</a><br
class="">
<br class="">
igor<br class="">
<br class="">
<br class="">
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
Igor Veresov
2018-11-29 00:59:23 UTC
Permalink
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
Post by s***@oracle.com
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
Post by Igor Veresov
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
Post by Igor Veresov
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
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
Post by Igor Veresov
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
Igor Veresov
2018-11-29 05:26:04 UTC
Permalink
Alright, how about the following solution: http://cr.openjdk.java.net/~iveresov/8193577/webrev.01/

igor
Post by Igor Veresov
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
Post by s***@oracle.com
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
Post by Igor Veresov
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
Post by Igor Veresov
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
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
Post by Igor Veresov
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
d***@oracle.com
2018-11-29 06:02:20 UTC
Permalink
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
Post by Igor Veresov
http://cr.openjdk.java.net/~iveresov/8193577/webrev.01/
igor
Post by Igor Veresov
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
Post by s***@oracle.com
Post by d***@oracle.com
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
Post by d***@oracle.com
dl
Post by Igor Veresov
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
Post by s***@oracle.com
On Nov 28, 2018, at 2:08 PM, Igor Veresov
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
Post by d***@oracle.com
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
Post by Igor Veresov
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
Igor Veresov
2018-11-29 06:57:21 UTC
Permalink
It would work right now. But I don’t want us fixing it again when somebody implements effectively final optimization in Graal.

igor
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
Alright, how about the following solution: http://cr.openjdk.java.net/~iveresov/8193577/webrev.01/ <http://cr.openjdk.java.net/~iveresov/8193577/webrev.01/>
igor
Post by Igor Veresov
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
Post by s***@oracle.com
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
Post by Igor Veresov
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
Post by Igor Veresov
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
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
Post by Igor Veresov
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
d***@oracle.com
2018-11-29 15:42:58 UTC
Permalink
OK your fix looks good.

dl
Post by Igor Veresov
It would work right now. But I don’t want us fixing it again when
somebody implements effectively final optimization in Graal.
igor
Post by d***@oracle.com
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
Post by Igor Veresov
http://cr.openjdk.java.net/~iveresov/8193577/webrev.01/
igor
Post by Igor Veresov
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
Post by s***@oracle.com
Post by d***@oracle.com
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
Post by d***@oracle.com
dl
Post by Igor Veresov
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
Post by s***@oracle.com
On Nov 28, 2018, at 2:08 PM, Igor Veresov
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
Post by d***@oracle.com
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
Post by Igor Veresov
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.
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
s***@oracle.com
2018-11-29 16:00:36 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Igor,<br>
<br>
+1<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 11/29/18 07:42, <a class="moz-txt-link-abbreviated" href="mailto:***@oracle.com">***@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:99965d21-854d-eaf7-8de6-***@oracle.com"> OK
your fix looks good.<br>
<br>
dl<br>
<br>
<div class="moz-cite-prefix">On 11/28/18 10:57 PM, Igor Veresov
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:A661D7FE-AE79-4D25-BE73-***@oracle.com">
<div class="">It would work right now. But I don’t want us
fixing it again when somebody implements effectively final
optimization in Graal.</div>
<br class="">
<div class=""> <span class="Apple-style-span">igor<br class="">
<br class="">
<br class="">
</span> </div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">On Nov 28, 2018, at 10:02 PM, <a
href="mailto:***@oracle.com" class=""
moz-do-not-send="true">***@oracle.com</a> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class=""> I like it better.  But do you really need
to use JNI to reset the values?  If you had<br class="">
<br class="">
<tt class="">static int POISON = 0x1234;  // not final</tt><tt
class=""><br class="">
</tt><tt class=""><br class="">
</tt><tt class="">class TaggedClass {</tt><tt class=""><br
class="">
</tt><tt class="">      static int field1 = 0xC0DE01 +
POISON;</tt><br class="">
<br class="">
in HeapFilter.java, is the compiler smart enough to
treat the value as constant until it changes?<br
class="">
<br class="">
dl<br class="">
<br class="">
<div class="moz-cite-prefix">On 11/28/18 9:26 PM, Igor
Veresov wrote:<br class="">
</div>
<blockquote type="cite"
cite="mid:9D54BD55-2C2D-40DF-96BD-***@oracle.com"
class="">
<div class="">Alright, how about the following
solution: <a
href="http://cr.openjdk.java.net/%7Eiveresov/8193577/webrev.01/"
class="" moz-do-not-send="true">http://cr.openjdk.java.net/~iveresov/8193577/webrev.01/</a></div>
<div class=""><br class="">
</div>
<div class=""> <span class="Apple-style-span">igor<br
class="">
<br class="">
<br class="">
</span> </div>
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Nov 28, 2018, at 4:59 PM, Igor
Veresov &lt;<a
href="mailto:***@oracle.com" class=""
moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">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.
<div class=""><br class="">
<div class=""> <span
class="Apple-style-span">igor<br
class="">
<br class="">
<br class="">
</span> </div>
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Nov 28, 2018, at 3:25
PM, <a
href="mailto:***@oracle.com"
class="" moz-do-not-send="true">***@oracle.com</a>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<div class="moz-cite-prefix">On
11/28/18 15:16, <a
class="moz-txt-link-abbreviated"
href="mailto:***@oracle.com" moz-do-not-send="true">***@oracle.com</a>
wrote:<br class="">
</div>
<blockquote type="cite"
cite="mid:e97be0a0-89ca-1a1e-3c4f-***@oracle.com"
class=""> 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.<br class="">
</blockquote>
<br class="">
I was thinking about the same.<br
class="">
Relaxing this just for Graal sounds
good.<br class="">
On the other hand, making the test
to know about Graal looks a little
bit strange. :)<br class="">
<br class="">
Thanks,<br class="">
Serguei <br class="">
<br class="">
<blockquote type="cite"
cite="mid:e97be0a0-89ca-1a1e-3c4f-***@oracle.com"
class=""> <br class="">
dl<br class="">
<br class="">
<div class="moz-cite-prefix">On
11/28/18 2:28 PM, Igor Veresov
wrote:<br class="">
</div>
<blockquote type="cite"
cite="mid:923D4C45-2684-41F6-9CD7-***@oracle.com"
class="">
<div class="">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.</div>
<div class=""><br class="">
</div>
<div class="">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.</div>
<div class=""><br class="">
</div>
<div class=""> <span
class="Apple-style-span">igor<br
class="">
<br class="">
<br class="">
</span> </div>
<div class=""><br class="">
<blockquote type="cite"
class="">
<div class="">On Nov 28,
2018, at 2:08 PM, Igor
Veresov &lt;<a
href="mailto:***@oracle.com"
class=""
moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:</div>
<br
class="Apple-interchange-newline">
<div class="">
<div class="">
<div class="">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.</div>
<br class="">
<div class=""> <span
class="Apple-style-span">igor<br
class="">
<br class="">
</span> </div>
<div class=""><br
class="">
<blockquote
type="cite" class="">
<div class="">On Nov
28, 2018, at 1:32
PM, <a
href="mailto:***@oracle.com"
class=""
moz-do-not-send="true">***@oracle.com</a>
wrote:</div>
<br
class="Apple-interchange-newline">
<div class="">
<div class="">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[].<br
class="">
<br class="">
dl<br class="">
<br class="">
On 11/28/18 1:13
PM, Igor Veresov
wrote:<br
class="">
<blockquote
type="cite"
class="">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.<br
class="">
<br class="">
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.<br
class="">
<br class="">
Webrev: <a
href="http://cr.openjdk.java.net/%7Eiveresov/8193577/webrev.00/"
class=""
moz-do-not-send="true">http://cr.openjdk.java.net/~iveresov/8193577/webrev.00/</a><br
class="">
JBS: <a
href="https://bugs.openjdk.java.net/browse/JDK-8193577"
class=""
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8193577</a><br
class="">
<br class="">
igor<br
class="">
<br class="">
<br class="">
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
Igor Veresov
2018-11-29 16:13:20 UTC
Permalink
Thanks Dean and Serguei!

igor
Post by s***@oracle.com
Hi Igor,
+1
Thanks,
Serguei
Post by d***@oracle.com
OK your fix looks good.
dl
Post by Igor Veresov
It would work right now. But I don’t want us fixing it again when somebody implements effectively final optimization in Graal.
igor
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
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
Post by Igor Veresov
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
Post by s***@oracle.com
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
Post by Igor Veresov
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
Post by Igor Veresov
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
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
Post by Igor Veresov
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
Loading...