RFR (S) JDK-8008962: NPG: Memory regression: One extra Monitor per ConstantPool
Ioi Lam
ioi.lam at oracle.com
Mon Mar 25 14:08:21 PDT 2013
Jetty numbers before/after my fix (JDK is 8/b81)
BEFORE
============================================================================
/scratch/iklam/jdk/tools/refworkload/130322/results.cplock_base
Benchmark Samples Mean Stdev Geomean Weight
footprint3_real 1 117032.00
jetty 1 117032.00
============================================================================
AFTER
============================================================================
/scratch/iklam/jdk/tools/refworkload/130322/results.cplock
Benchmark Samples Mean Stdev Geomean Weight
footprint3_real 1 116728.00
jetty 1 116728.00
============================================================================
So saving of about 300K.
Also, for curiosity, I have tested jetty for the promoted JDK8 builds
from the past 6 months -- from /java/re, linux_amd64:
b57, according to the bug report 8001590, is the last version that did
not have NPG
b78 has a big regression. It's fixed somewhat in b82 but still we are
much worse than b57.
==========================================================================
Samples size stddev date
b55 6 96644.67 27.18 09/06/2012
b56 6 97334.00 26.50 09/13/2012
b57 6 97326.67 73.13 09/20/2012 << Last w/o NPG
b58 6 103212.00 70.70 09/27/2012
b59 6 103220.67 46.61 10/03/2012
b60 6 103187.33 102.68 10/11/2012
b61 6 100036.00 79.44 10/18/2012
b62 6 100020.67 78.77 10/25/2012
b63 6 100323.33 135.85 11/01/2012
b64 6 100297.33 76.72 11/08/2012
b65 6 101616.67 70.63 11/15/2012
b66 6 101445.33 71.63 11/29/2012
b67 6 101135.33 80.26 12/06/2012
b68 6 101596.00 89.66 12/13/2012
b69 6 101644.00 72.62 12/20/2012
b70 6 101716.00 106.19 12/27/2012
b71 6 101852.00 119.71 01/03/2013
b72 6 101844.67 144.12 01/10/2013
b73 6 102121.33 83.20 01/16/2013
b74 6 102473.33 61.43 01/24/2013
b75 6 101633.33 61.54 01/31/2013
b76 6 101760.00 65.73 02/07/2013
b77 6 101188.00 45.75 02/14/2013
b78 6 117358.67 154.79 02/21/2013 << big regression
b79 6 117236.67 58.97 02/28/2013
b80 6 117454.00 198.63 03/07/2013
b81 6 117308.67 30.61 03/14/2013
b82 6 106822.00 280.24 03/21/2013
==========================================================================
- Ioi
On 03/21/2013 10:46 PM, David Holmes wrote:
> This looks okay to me. Do we have updated Jetty figures to show the
> memory regression has gone/reduced?
>
> Aside: Your webrev frames view is broken - the navigation frame gives
> a 404 error.
>
> Thanks,
> David
>
> On 22/03/2013 3:12 PM, Ioi Lam wrote:
>> Hi folks,
>>
>> I have updated the patch. Please review
>>
>> http://cr.openjdk.java.net/~iklam/8008962/constpool_lock_002/
>> <http://cr.openjdk.java.net/%7Eiklam/8008962/constpool_lock_002/>
>>
>> The only change is to check if the lock is not yet initialized. This
>> happens only during class file parsing, so locking is not necessary.
>>
>> oop cplock = this_oop->lock();
>> ObjectLocker ol(cplock , THREAD, cplock != NULL);
>>
>> Thanks
>> - Ioi
>>
>>
>> On 03/18/2013 09:32 AM, Ioi Lam wrote:
>>> 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
>>
More information about the hotspot-runtime-dev
mailing list