<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 <<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 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 <<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="">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>