Request Review: 6950075: nmethod sweeper should operate concurrently

[Eric Caspole]

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