RFR (S) JDK-8008962: NPG: Memory regression: One extra Monitor per ConstantPool
David Holmes
david.holmes at oracle.com
Sun Mar 17 15:53:47 PDT 2013
On 18/03/2013 3:48 AM, Coleen Phillimore wrote:
> On 3/17/2013 1:43 PM, Ioi Lam wrote:
>> I have a kind of related questions: if locking of an oop is contended,
>> do we allocate a Monitor object for this oop? Will the Monitor be
>> eventually freed even if the oop is sill alive?
>
> I don't know if the object monitor is deflated, or when.
If the int[0] is treated as a normal object then we may deflate the
object-monitor as we do for any other object: if they are idle during
safepoint cleanup. We don't deallocate object-monitors ever.
Okay so summing this up:
- the init_lock is a int[0]
- if we don't need to inflate (do we have any stats on this?) then we
don't get any overhead beyond the int[0]
- if we do inflate then we may also deflate later; further as those
object-monitors come from the pool we may not actually have to create one
In contrast the explicit Monitor allocation for the CP, explicitly
allocated a Monitor.
Alright I'm convinced that re-using the init_lock is likely to be better.
Note this isn't (yet) a Review as I haven't studied the code yet.
Thanks,
David
-----
>>
>> The old code in constantPool.cpp allocated a Monitor, not a Mutex:
>>
>> set_lock(new Monitor(Monitor::nonleaf + 2, "A constant pool lock"));
>
> Yes, I think it should have been a Mutex* because there's no
> wait/notify. They're both the the same size.
> Coleen
>
>>
>> Thanks
>> - Ioi
>>
>> On 03/17/2013 10:33 AM, Coleen Phillimore wrote:
>>>
>>> Hi David,
>>>
>>> You reviewed the init_lock change. I'm not sure if you reviewed the
>>> Mutex* that we added for the constant pool change as well. Both added
>>> footprint relative to permgen. You were pretty dubious about
>>> zeroing out the init_lock too iirc. More below:
>>>
>>> On 3/17/2013 3:43 AM, David Holmes wrote:
>>>> On 17/03/2013 2:04 PM, Ioi Lam wrote:
>>>>> 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.
>>>>
>>>> I understand, but the need for a new lock object is problematic.
>>>> With the permgen removal the klass stopped being an oop and a new
>>>> object had to be introduced just for class initialization locking.
>>>> That was bad for dynamic footprint but we compromised by making the
>>>> lock object short-lived. Then we needed a lock-object for the CP.
>>>> This originally introduced a new lock object and that caused this
>>>> footprint issue. So you are proposing to re-use the init-lock, but
>>>> given that was short-lived and will now be long lived, you have not
>>>> made any net gain in footprint in the long term.
>>>
>>> Relative to when we had permgen, this isn't a net gain, but the
>>> init_lock is much smaller than the Mutex* that we have in the
>>> Constant Pool. If uncontended the init_lock is only 3 words, the
>>> mutex is something like 156 bytes. So we have made net gain relative
>>> to the current sources.
>>>
>>>>
>>>>>>> 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.
>>>>
>>>> The question is more, can they be called before or during class
>>>> initialization?
>>>>
>>>
>>> A good test would be to call class.forName in the <clinit>
>>> function. This lock can be taken during initialization but if it's
>>> a reentrant lock, it should be okay. I'm sure at least hundreds of
>>> tests already do this, but verifying this with a specific test and
>>> making sure would be a good thing to do.
>>>
>>> Coleen
>>>>>> 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.
>>>>
>>>> Okay. But recursive locking can also be problematic if you don't
>>>> fully understand the circumstances under which it can occur -
>>>> because you effectively lose atomicity relative to actions in the
>>>> current thread.
>>>>
>>>> David
>>>> -----
>>>>
>>>>> 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
>>>>>
>>>
>>
>
More information about the hotspot-runtime-dev
mailing list