RFR (S) JDK-8008962: NPG: Memory regression: One extra Monitor per ConstantPool

[Ioi Lam]

On 03/16/2013 04:37 PM, David Holmes wrote:
> Hi Ioi,
>
> On 16/03/2013 5:07 PM, Ioi Lam wrote:
>> Please review:
>> http://cr.openjdk.java.net/~iklam/8008962/constpool_lock_001/
>> <http://cr.openjdk.java.net/%7Eiklam/8008962/constpool_lock_001/>
>>
>> Bug: 8008962: NPG: Memory regression: One extra Monitor per ConstantPool
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8008962
>> https://jbs.oracle.com/bugs/browse/JDK-8008962 (Oracle Internal)
>>
>> Summary of fix:
>>
>> The fix is implemented according to Coleen's suggestion in the bug's
>> comments.
>>
>> Instead of using a Monitor for each ConstantPool (~150 bytes per
>> class), I re-use InstanceKlass::init_lock() for locking the
>> ConstantPool.
>>
>> init_lock used to be cleared after the class had been initialized.
>> There was complicated code that needed to deal with the clearing
>> (using volatile, OrderAccess::storestore(), etc). This code is no
>> longer necessary and has been removed.
>
> But the whole point of removing it was that we did not want the extra 
> memory usage to exist for the lifetime of the class! This will 
> increase dynamic footprint in a potentially unacceptable way.
>
We need a lock for the lifetime of the ConstantPool (see below), so I am 
just re-using the init_lock and make it live forever. If I remove the 
init_lock then I have to create another lock object.

>>      The overhead is reduced to one int[0] per class.
>>
>> Because the lock is used at two different levels (class
>> initialization, and ConstantPool access), I thought about the
>> possibility of deadlocks.
>
> Top understand the deadlock potential I need to understand what locks 
> the constant pool and why/when. 

There are various places such as ConstantPool::klass_at_impl that need 
to make atomic modifications of an CP entry and its corresponding tag. 
These can be called well after the class has finished initialization.

> Is there a possibility of a self-deadlock if during class 
> initialization we have to lock the constant-pool ourselves?
The locking is done using ObjectLocker with an oop, so it is self 
reentrant, just like a regular Java monitor entry. Unlike mutexes, there 
won't be self deadlocks.

Thanks
- Ioi
>
> Thanks,
> David
> ------
>
>
>> However,
>>      it seems the only case of a deadlock would be some sort of circular
>> dependency
>>      in running <clinit> (as in the following example). However, in this
>> case, a
>>      deadlock would have already happened, just from grabbing the
>> initialization
>>      locks, before we would even have a chance to access the
>> ConstantPools. Hence,
>>      reusing the lock for ConstantPool access doesn't add any new paths
>> for deadlocks.
>>
>>      -------------------------------
>>      public class ConcurClinit {
>>          static void sleep(int ms) {
>>              try {Thread.sleep(ms);} catch (Throwable t) {;}
>>          }
>>          static class A {
>>              static {
>>                  sleep(1000);
>>                  B.xxx();
>>                  sleep(1000);
>>              }
>>              static void xxx() {}
>>          }
>>          static class B {
>>              static {
>>                  sleep(1000);
>>                  A.xxx();
>>                  sleep(1000);
>>              }
>>              static void xxx() {}
>>          }
>>          public static void main(String args[]) {
>>              (new Thread() {
>>                      public void run() {
>>                          A a = new A();
>>                      }
>>                  }).start();
>>              (new Thread() {
>>                      public void run() {
>>                          B b = new B();
>>                      }
>>                  }).start();
>>          }
>>      }
>>      -------------------------------
>>
>> Tests executed:
>>
>>      * JPRT
>>      * JCK (vm, lang, api/signaturetest)
>>      * UTE (vm.runtime.testlist, vm.quick.testlist,
>> vm.parallel_class_loading.testlist
>> )
>>
>> Thanks,
>> Ioi