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

[Ioi Lam]

On 03/17/2013 12:43 AM, David Holmes wrote:
>
>> 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?
>

Klass::init_lock is initialized in ClassFileParser::parseClassFile(). 
However, the CP is created before this. So there's a chance that the CP 
may try to lock on ConstantPool::lock() before Klass::init_lock() is 
initialized (or even before ConstantPool::_pool_holder is initialized).

Nevertheless, I have not (yet) seen this happening with a fair amount of 
stress tests.

Also, up to the initialization of Klass::init_lock(), only the 
ClassFileParser has a reference to the InstanceKlass and the 
ConstantPool, so everything is single threaded. I will change the code 
to be something like this (similar to what was done in InstanceKlass 
with the init_lock):

      oop cplock = lock();
      ObjectLocker ol(cplock, THREAD, cplock != NULL);

> - 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] 

I don't have any stats. How would one go about collecting the locking 
stats on specific objects?

Looking at the code, most use of the lock would be in 
ConstantPool::klass_at_impl(), and only if the slot is still an 
unresolved class. Also, the lock is usually held for a very short period 
of time, unless you hit an exception, or hit a GC at this block

       MonitorLockerEx ml(this_oop->lock());
       // Only updated constant pool - if it is resolved.
       do_resolve = this_oop->tag_at(which).is_unresolved_klass();
       if (do_resolve) {
         ClassLoaderData* this_key = 
this_oop->pool_holder()->class_loader_data();
         this_key->record_dependency(k(), CHECK_NULL); // Can throw OOM  
<<<<<< GC may happen here
         this_oop->klass_at_put(which, k());
       }

So my wild guess is you rarely would get a contention on the lock.

>>> 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.

Sorry I don't quote understand this. Could you explain more?

Thanks a lot!

- Ioi