Thanks,
Yasumasa
David
On 21/09/2017 8:44 AM, Yasumasa Suenaga wrote:
Hi David,
jdk10/hs has been opened [1].
Could you push this change?
Thanks,
Yasumasa
[1]
http://mail.openjdk.java.net/pipermail/jdk10-dev/2017-September/000499.html
<http://mail.openjdk.java.net/pipermail/jdk10-dev/2017-September/000499.html>
On 2017/09/19 12:31, David Holmes wrote:
On 19/09/2017 1:19 PM, Yasumasa Suenaga wrote:
Thanks David,
BTW, can I push this change after jdk10/master is opened?
I cannot access JPRT.
I think we'd probably prefer this to go into jdk10/hs - once
it is open - and for that you need a sponsor.
Thanks,
David
Yasumasa
2017/09/19 午後0:08 "David Holmes"
<david.hol...@oracle.com
<mailto:david.hol...@oracle.com>
<mailto:david.hol...@oracle.com
<mailto:david.hol...@oracle.com>>>:
Hi Yasumasa,
On 19/09/2017 12:55 PM, Yasumasa Suenaga wrote:
Thanks Chris, Robbin,
I'm waiting reviewer(s) for this change.
Reviewed.
This simply reverts the change of 8185102.
Thanks,
David
-----
Yasumasa
2017/09/19 午前7:14 "Chris Plummer"
<chris.plum...@oracle.com
<mailto:chris.plum...@oracle.com>
<mailto:chris.plum...@oracle.com
<mailto:chris.plum...@oracle.com>>
<mailto:chris.plum...@oracle.com
<mailto:chris.plum...@oracle.com>
<mailto:chris.plum...@oracle.com
<mailto:chris.plum...@oracle.com>>>>:
Hi Yasumasa,
Ok, I see now that CIntegerField is just
an interface, so
it's up to
a class to implement getValue() to fetch
the field. I'm a bit
unclear on how that part works, but from
responses by
others, it
seems this is ok.
I've run all the tests I can find that use
jstack or jhsdb,
and the
assert was not triggered. Probably need to
have a NMethod
on the
stack to trigger the code you are fixing.
thanks,
Chris
On 9/17/17 1:13 AM, Yasumasa Suenaga wrote:
Hi Chris,
I've tested this issue on Fedora 26
x86_64.
I think we can sue CIntegerField at
this point because
CIntegerField is not specialized for
various int size [1].
In fact, CIntegerField had been used
at this point [2],
and HSDB
worked fine.
Thanks,
Yasumasa
[1]
http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29
<http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29>
<http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29
<http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29>>
<http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29
<http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29>
<http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29
<http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29>>>
[2]
http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3
<http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3>
<http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3
<http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3>>
<http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3
<http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3>
<http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3
<http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3>>>
On 2017/09/17 3:58, Chris Plummer wrote:
Hi Yasumasa,
Is this on a 32-bit system? I
don't see how you could
otherwise call getCIntegerField()
on a long type.
jlong is
always 64-bit and long is
(generally) 32-bit on 32-bit
systems, and 64-bit on 64-bit
systems, at least
that seems
to be the case with linux.
From what I can see,
_stack_traversal_mark is now
the only
long type in vmStructs.cpp. I
don't know that we have a
mechanism to safely fetch it on
both 32-bit and
64-bit systems.
_stack_traversal_mark seems to be
a long because
_traversals
is also a long.
static long
_traversals; //
Stack scan count, also sweep ID.
This too might be considered a
bug. I'm not sure
why you
would want the size of this field
to vary between
32-bit and
64-bit systems (adding
compiler-dev to help answer
that).
So, while I would agree that your
fix is generally
in the
right direction, I think we first
need to revisit
the use of
long for these fields. If they can
be changed to an
int,
then your fix is correct (pending
the changes to
int). If
not, then maybe we need
getCLongField() support.
And lastly, we really should have
a test to detect
this bug.
Maybe we already do, and it is
failing but is going
unnoticed for some reason. I'll
try to look into
that some
more on Monday.
thanks,
Chris
On 9/16/17 5:20 AM, Yasumasa
Suenaga wrote:
Hi all,
I tried to get thread dump via
jstack command
on CLHSDB.
But it was failed as below:
```
Caused by:
sun.jvm.hotspot.types.WrongTypeException:
field "_stack_traversal_mark"
in type nmethod
is not of
type jlong, but instead of
type long
at
jdk.hotspot.agent/sun.jvm.hotspot.types.basic.BasicType.getField(BasicType.java:206)
at
jdk.hotspot.agent/sun.jvm.hotspot.types.basic.BasicType.getField(BasicType.java:212)
at
jdk.hotspot.agent/sun.jvm.hotspot.types.basic.BasicType.getJLongField(BasicType.java:249)
at
jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod.initialize(NMethod.java:108)
at
jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod.access$000(NMethod.java:35)
at
jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod$1.update(NMethod.java:81)
at
jdk.hotspot.agent/sun.jvm.hotspot.runtime.VM.registerVMInitializedObserver(VM.java:451)
at
jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod.<clinit>(NMethod.java:79)
... 23 more
```
I think this exception is
caused by JDK-8186837.
This changeset has changed the
type of
`nmethod::_stack_traversal_mark` to `long` from
`jlong`.
SA should follow this change.
I uploaded a webrev for this
issue. This webrev is
generated from consolidated
repo (jdk10/master).
Could you review it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/
<http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/>
<http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/
<http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/>>
<http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/
<http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/>
<http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/
<http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/>>>
I cannot access JPRT. So I
need reviewer.
Thanks,
Yasumasa