RFR (S) JDK-8008962: NPG: Memory regression: One extra Monitor per ConstantPool
Ioi Lam
ioi.lam at oracle.com
Fri Apr 12 11:52:19 PDT 2013
Hi,
I have updated the patch to reflect comments I received from Karen offline:
http://cr.openjdk.java.net/~iklam/8008962/constpool_lock_003/
<http://cr.openjdk.java.net/%7Eiklam/8008962/constpool_lock_003/>
The changes from the last version are only comments and naming of
fileds/methods. No programmatic changes.
Thanks
- Ioi
On 03/25/2013 02:08 PM, Ioi Lam wrote:
> 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
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130412/04953e3e/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list