RFR (S) 8013942: JSR 292: assert(type() == T_OBJECT) failed: type check

[serguei.spitsyn at oracle.com]

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 at oracle.com wrote:
> On 6/27/14 1:34 AM, serguei.spitsyn at 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 at openjdk.java.net  Developers;serviceability-dev at 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 at 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
>>>>>
>>
>