[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR (S) 8013942: JSR 292: assert(type() == T_OBJECT) failed: type check
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2014-06-30 21:09:13
Message-ID: 53B1D1F9.5050404 () oracle ! com
[Download RAW message or body]
Coleen, Vladimir and Markus,
Markus raised a good point on the predefined number of words reserved
for inlined mask storage.
I'm suggesting the following update:
diff -r 6b78c6948ec8 src/share/vm/interpreter/oopMapCache.hpp
--- a/src/share/vm/interpreter/oopMapCache.hpp Wed Jun 25 22:12:25 2014
+0000
+++ b/src/share/vm/interpreter/oopMapCache.hpp Mon Jun 30 14:02:17 2014
-0700
@@ -66,19 +66,15 @@ class InterpreterOopMap: ResourceObj {
public:
enum {
- N = 2, // the number of words reserved
+ N = 4, // the number of words reserved
// for inlined mask storage
small_mask_limit = N * BitsPerWord, // the maximum number of bits
// available for small masks,
// small_mask_limit can be
set to 0
// for testing bit_mask
allocation
The updated webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8013942-JVMTI-lobj.2
Please, let me know if this update looks Ok.
Thanks,
Serguei
On 6/27/14 1:53 AM, serguei.spitsyn@oracle.com wrote:
> On 6/27/14 1:34 AM, serguei.spitsyn@oracle.com wrote:
> > Hi Markus,
> >
> > I raised a good point, thanks!
>
> Sorry, I wanted to say: "You raised a good point!" :)
> > What do you think about increasing the predefined size (N from 2 to 4)?
> >
> > 64 class InterpreterOopMap: ResourceObj {
> > 65 friend class OopMapCache;
> > 66
> > 67 public:
> > 68 enum {
> > 69 N = 2, // the number of words reserved
> > 70 // for inlined mask storage
> > 71 small_mask_limit = N * BitsPerWord, // the maximum number of bits
> > 72 // available for small masks,
> > 73 // small_mask_limit can be set to 0
> > 74 // for testing bit_mask allocation
> >
> >
> > Thanks,
> > Serguei
> >
> > On 6/27/14 1:12 AM, Markus Grönlund wrote:
> > > Hi Serguei,
> > >
> > >
> > > I am not convinced this is the right way to do this - by removing the #ifdef \
> > > ENABLE_ZAP_DEAD_LOCALS" (which is an COMPILER2 specific #define under an \
> > > ASSERT), we have now unconditionally increased the bitsize for every oopmap \
> > > entry to two (compared to one previously) - the inlined oop_map bits_size cache \
> > > is predefined to hold two words.
> > > This means the oopmap bitmap cache is effectively halved unconditionally.
> > >
> > > /Markus
> > >
> > >
> > > -----Original Message-----
> > > From: Serguei Spitsyn
> > > Sent: den 27 juni 2014 00:15
> > > To: Vladimir Ivanov;hotspot-dev@openjdk.java.net \
> > > Developers;serviceability-dev@openjdk.java.net
> > > Subject: Re: RFR (S) 8013942: JSR 292: assert(type() == T_OBJECT) failed: type \
> > > check
> > > Thanks, Vladimir!
> > > Serguei
> > >
> > > On 6/26/14 3:02 PM, Vladimir Ivanov wrote:
> > > > Looks good.
> > > >
> > > > Best regards,
> > > > Vladimir Ivanov
> > > >
> > > > On 6/25/14 5:57 PM,serguei.spitsyn@oracle.com wrote:
> > > > > Please, review the fix for:
> > > > > https://bugs.openjdk.java.net/browse/JDK-8013942
> > > > >
> > > > >
> > > > > Open webrev:
> > > > > http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8013942-JVM
> > > > > TI-lobj.1
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Summary:
> > > > >
> > > > > This is a Nightly Stabilization issue.
> > > > >
> > > > > The problem is that the JVMTI GetLocalObject() function is hitting
> > > > > the assert
> > > > > as the type of the local at current bci is not T_OBJECT as expected.
> > > > > It is because this local is alive only in a narrow scope of one
> > > > > condition in the method but current bci is out of this csope.
> > > > >
> > > > > A fragment from the target method:
> > > > >
> > > > > static Class<?> canonicalize(Class<?> t, int how) {
> > > > > Class<?> ct; <== The local
> > > > > if (t == Object.class) {
> > > > > // no change, ever
> > > > > } else if (!t.isPrimitive()) {
> > > > > switch (how) {
> > > > > case UNWRAP:
> > > > > ct = Wrapper.asPrimitiveType(t); <== Initialized
> > > > > here
> > > > > if (ct != t) return ct;
> > > > > break;
> > > > > case RAW_RETURN:
> > > > > case ERASE:
> > > > > return Object.class;
> > > > > }
> > > > > } else if (t == void.class) { <== The
> > > > > GetLocalObject() is called with bci in this block
> > > > > // no change, usually
> > > > > switch (how) {
> > > > > case RAW_RETURN:
> > > > > return int.class;
> > > > > case WRAP:
> > > > > return Void.class;
> > > > > }
> > > > > } else {
> > > > > . . .
> > > > >
> > > > > The local 'ct' is only alive in the block of condition "if
> > > > > (!t.isPrimitive())".
> > > > > However, it is dead local in the next block.
> > > > >
> > > > > The fix suggested in the webrev above is to use the oop_mask for
> > > > > the method's current bci.
> > > > > It tells when the local is dead in the scope of this bci.
> > > > > Return the JVMTI_ERROR_INVALID_SLOT error in such a case.
> > > > >
> > > > > The fix also includes the tweeks which are necessary to enable the
> > > > > InterpreterOopMap.is_dead()
> > > > > function in the product mode as it was originally defined only
> > > > > under "#ifdef ENABLE_ZAP_DEAD_LOCALS".
> > > > > This makes the code in the oopMapCache.?pp a little bit more simple.
> > > > >
> > > > >
> > > > > Testing:
> > > > > Running the failing tests: vm.mlvm.indy.func.jvmti
> > > > > In progress: nsk.jvmti, nsk.jdi, nsk.jdwp test runs on sparcv9 and
> > > > > amd64
> > > > >
> > > > >
> > > > > Thanks,
> > > > > Serguei
> > > > >
> >
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Coleen, Vladimir and Markus,<br>
<br>
Markus raised a good point on the predefined number of words
reserved for inlined mask storage.<br>
<br>
I'm suggesting the following update:<br>
<br>
diff -r 6b78c6948ec8 src/share/vm/interpreter/oopMapCache.hpp<br>
--- a/src/share/vm/interpreter/oopMapCache.hpp Wed Jun 25
22:12:25 2014 +0000<br>
+++ b/src/share/vm/interpreter/oopMapCache.hpp Mon Jun 30
14:02:17 2014 -0700<br>
@@ -66,19 +66,15 @@ class InterpreterOopMap: ResourceObj {<br>
<br>
public:<br>
enum {<br>
- \
N \
= 2, \
// the number of words reserved<br>
+ \
N \
= 4, \
// the number of words reserved<br>
&n \
bsp; \
// for inlined mask storage<br>
small_mask_limit = N * BitsPerWord, // the \
maximum number of bits<br>
&n \
bsp; \
// available for small masks,<br>
&n \
bsp; \
// small_mask_limit can be set to 0<br>
&n \
bsp; \
// for testing bit_mask allocation<br>
<br>
<br>
The updated webrev:<br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8013942-JVMTI-lobj.2"> \
http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8013942-JVMTI-lobj.2</a><br>
<br>
<br>
Please, let me know if this update looks Ok.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
On 6/27/14 1:53 AM, <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote:<br> \
</div> <blockquote cite="mid:53AD310E.1010200@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 6/27/14 1:34 AM, <a
moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote cite="mid:53AD2C8F.9090702@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Markus,<br>
<br>
I raised a good point, thanks!<br>
</div>
</blockquote>
<br>
Sorry, I wanted to say: "You raised a good point!" :)<br>
<blockquote cite="mid:53AD2C8F.9090702@oracle.com" type="cite">
<div class="moz-cite-prefix"> What do you think about increasing
the predefined size (N from 2 to 4)?<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre> 64 class InterpreterOopMap: ResourceObj {
65 friend class OopMapCache;
66
67 public:
68 enum {
69 N = 2, // the number of words reserved
70 // for inlined mask storage
71 small_mask_limit = N * BitsPerWord, // the maximum number of bits
72 // available for small masks,
73 // small_mask_limit can be set to 0
74 // for testing bit_mask \
allocation</pre> <br>
<br>
Thanks,<br>
Serguei<br>
<br>
On 6/27/14 1:12 AM, Markus Grönlund wrote:<br>
</div>
<blockquote
cite="mid:b0621477-b209-46db-8678-9b6a24cb87ce@default"
type="cite">
<pre wrap="">Hi Serguei,
I am not convinced this is the right way to do this - by removing the #ifdef \
ENABLE_ZAP_DEAD_LOCALS" (which is an COMPILER2 specific #define under an ASSERT), we \
have now unconditionally increased the bitsize for every oopmap entry to two \
(compared to one previously) - the inlined oop_map bits_size cache is predefined to \
hold two words.
This means the oopmap bitmap cache is effectively halved unconditionally.
/Markus
-----Original Message-----
From: Serguei Spitsyn
Sent: den 27 juni 2014 00:15
To: Vladimir Ivanov; <a moz-do-not-send="true" class="moz-txt-link-abbreviated" \
href="mailto:hotspot-dev@openjdk.java.net">hotspot-dev@openjdk.java.net</a> \
Developers; <a moz-do-not-send="true" class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a>
Subject: Re: RFR (S) 8013942: JSR 292: assert(type() == T_OBJECT) failed: type check
Thanks, Vladimir!
Serguei
On 6/26/14 3:02 PM, Vladimir Ivanov wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Looks good.
Best regards,
Vladimir Ivanov
On 6/25/14 5:57 PM, <a moz-do-not-send="true" class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote: </pre>
<blockquote type="cite">
<pre wrap="">Please, review the fix for:
<a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8013942">https://bugs.openjdk.java.net/browse/JDK-8013942</a>
Open webrev:
<a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2014/hotspot/8013942-JVM">http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8013942-JVM</a>
TI-lobj.1
Summary:
This is a Nightly Stabilization issue.
The problem is that the JVMTI GetLocalObject() function is hitting
the assert
as the type of the local at current bci is not T_OBJECT as expected.
It is because this local is alive only in a narrow scope of one
condition in the method but current bci is out of this csope.
A fragment from the target method:
static Class<?> canonicalize(Class<?> t, int how) {
Class<?> ct; <== The local
if (t == Object.class) {
// no change, ever
} else if (!t.isPrimitive()) {
switch (how) {
case UNWRAP:
ct = Wrapper.asPrimitiveType(t); <== Initialized
here
if (ct != t) return ct;
break;
case RAW_RETURN:
case ERASE:
return Object.class;
}
} else if (t == void.class) { <== The
GetLocalObject() is called with bci in this block
// no change, usually
switch (how) {
case RAW_RETURN:
return int.class;
case WRAP:
return Void.class;
}
} else {
. . .
The local 'ct' is only alive in the block of condition "if
(!t.isPrimitive())".
However, it is dead local in the next block.
The fix suggested in the webrev above is to use the oop_mask for
the method's current bci.
It tells when the local is dead in the scope of this bci.
Return the JVMTI_ERROR_INVALID_SLOT error in such a case.
The fix also includes the tweeks which are necessary to enable the
InterpreterOopMap.is_dead()
function in the product mode as it was originally defined only
under "#ifdef ENABLE_ZAP_DEAD_LOCALS".
This makes the code in the oopMapCache.?pp a little bit more simple.
Testing:
Running the failing tests: vm.mlvm.indy.func.jvmti
In progress: nsk.jvmti, nsk.jdi, nsk.jdwp test runs on sparcv9 and
amd64
Thanks,
Serguei
</pre>
</blockquote>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic