Fwd: Request Review: 6950075: nmethod sweeper should operate concurrently

[Tom Rodriguez]

For some reason this bounced earlier.

tom

Begin forwarded message:

> From: Tom Rodriguez <tom.rodriguez at oracle.com>
> Date: May 18, 2010 11:16:26 AM PDT
> To: Eric Caspole <eric.caspole at amd.com>
> Cc: "hotspot-dev at openjdk.java.net Developers" <hotspot-dev at openjdk.java.net>
> Subject: Re: Request Review: 6950075: nmethod sweeper should operate concurrently
> 
> I pushed these changes last night.  Thanks!
> 
> tom
> 
> On May 12, 2010, at 10:10 AM, Eric Caspole wrote:
> 
>> Hi Tom,
>> I updated the comments in rev 11, let me know what you think.
>> 
>> http://cr.openjdk.java.net/~ecaspole/concswpr/webrev.11/webrev/
>> 
>> Thanks,
>> Eric
>> 
>> 
>> On May 7, 2010, at 11:23 AM, Tom Rodriguez wrote:
>> 
>>> There's still some language in sweeper.cpp about running at a safepoint.  I know scan_stacks still does but the other parts don't.  In particular this comment:
>>> 
>>>     // long time we don't want to keep rescanning at every safepoint.
>>> 
>>> should lose the "at every safepoint" part.  This needs updating:
>>> 
>>> // Shut off compiler. Sweeper will run exiting from this safepoint
>>> // and turn it back on if it clears enough space
>>> 
>>> and this:
>>> 
>>> if (UseCodeCacheFlushing) {
>>>   if (!CodeCache::needs_flushing()) {
>>>     // In a safepoint, no race with setters
>>>     _advise_to_sweep = 0;
>>>   }
>>> 
>>> probably needs some sort of fix.
>>> 
>>> Otherwise this looks good.
>>> 
>>> tom
>>> 
>>> On May 7, 2010, at 9:27 AM, Eric Caspole wrote:
>>> 
>>>> Thanks Vladimir, see rev 10:
>>>> 
>>>> http://cr.openjdk.java.net/~ecaspole/concswpr/webrev.10/
>>>> 
>>>> Eric
>>>> 
>>>> 
>>>> On May 5, 2010, at 11:51 PM, Vladimir Kozlov wrote:
>>>> 
>>>>> Eric,
>>>>> 
>>>>> It looks good. Just minor notes.
>>>>> 
>>>>> 
>>>>> Extend comment for your additional conditions:
>>>>> 
>>>>> !   if ((_current == NULL && !_rescan) && !(UseCodeCacheFlushing && !CompileBroker::should_compile_new_jobs())) {
>>>>> 
>>>>> 
>>>>> Put first call javaTimeMillis under flag:
>>>>> 
>>>>> +   jlong sweep_start;
>>>>> +   if(PrintMethodFlushing)
>>>>> +     sweep_start = os::javaTimeMillis();
>>>>> 
>>>>> Thanks,
>>>>> Vladimir
>>>>> 
>>>>> 
>>>>> On 5/5/10 6:03 PM, Eric Caspole wrote:
>>>>>> In the current JVM, the nmethod sweeper runs during each safepoint,
>>>>>> and may return early if there is no work to do. It periodically scans
>>>>>> the thread stacks to determine if not-entrant nmethods have no
>>>>>> lingering active stack frames and can be converted to zombies. It
>>>>>> then walks the code cache promoting not-entrant nmethods to zombies
>>>>>> if possible and deleting the zombies. It walks a fraction of the code
>>>>>> cache in each turn, set by NmethodSweepFraction, which is 4 by
>>>>>> default. When the whole code cache is traversed and no progress can
>>>>>> be made, the sweeper will go idle until another nmethod becomes not-
>>>>>> entrant, starting the process over again.
>>>>>> 
>>>>>> In applications with a lot of compiled code, walking the code cache
>>>>>> can take many milliseconds, contributing to long safepoint pause
>>>>>> times, reducing application throughput. This change makes part of the
>>>>>> sweeper work concurrent to reduce the safepoint pause time.
>>>>>> 
>>>>>> In this webrev, the stack scans continue to happen at safepoint time
>>>>>> but the code cache traversal is concurrent with the application. The
>>>>>> traversal work runs on the compiler threads where they will check to
>>>>>> sweep each time passing through the compile queue. They may take the
>>>>>> code cache and stub related locks as necessary to do the sweeping
>>>>>> safely. Only one thread will sweep at a time and it preserves the
>>>>>> existing way of traversing in chunks by NmethodSweepFraction.
>>>>>> 
>>>>>> There are a few adjustments to the checks regarding the code cache
>>>>>> unloading feature to accommodate these changes. Also I added new
>>>>>> convenience methods such as CodeCache::next_nmethod() for simple
>>>>>> traversal.
>>>>>> 
>>>>>> I worked out this design with Tom Rodriguez over the last month or so.
>>>>>> We'd like some other eyes on it.
>>>>>> 
>>>>>> The webrev is at
>>>>>> http://cr.openjdk.java.net/~ecaspole/concswpr/webrev.09/webrev/
>>>>>> 
>>>>>> Thanks,
>>>>>> Eric
>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>>> 
>>> 
>>> 
>> 
>> 
>