RFR (S): Avoid evacuation if concurrent GC was cancelled

[Roman Kennke]

Ah yes I see what you mean. Yes we can change to assert_locked_or_safepoint() there.

/Roman

Am 05.12.2016 6:44 nachm. schrieb Zhengyu Gu <zgu at redhat.com>:
>
> On 12/05/2016 12:28 PM, Roman Kennke wrote: 
>
> > Am Montag, den 05.12.2016, 12:25 -0500 schrieb Zhengyu Gu: 
> >> 114 // b. Cancel evacuation, if in progress 
> >> 115 if (_heap->is_evacuation_in_progress()) { 
> >> 116 MutexLocker mu(Threads_lock); 
> >> 117 _heap->set_evacuation_in_progress(false); 
> >> 118 } 
> >> 
> >> 
> >> I think that we can eliminate Threads_lock above by changing the 
> >> assertion below: 
> >> 
> >> void JavaThread::set_evacuation_in_progress_all_threads(bool in_prog) 
> >> { 
> >>     assert(Threads_lock->owned_by_self(), "must hold 
> >> Threads_lock");       <==== assert_locked_or_safepoint(Threads_lock) 
> >>     _evacuation_in_progress_global = in_prog; 
> >>     for (JavaThread* t = Threads::first(); t != NULL; t = t->next()) { 
> >>       t->set_evacuation_in_progress(in_prog); 
> >>     } 
> >> } 
> > No, I don't think so. We're iterating over the threads, so we should 
> > hold that lock. However, as I mentioned in that other email, the 
> > VMThread should already hold it. Now that I think about it again, it's 
> > probably not going to deadlock, it's simply re-entrant. In any case, 
> > acquiring it should not be necessary. 
>
> I think that it is safe to iterate over the thread list without the Thread_lock during safepoint. 
>
> Check following code: 
>
> void Threads::threads_do(ThreadClosure* tc) { 
>    assert_locked_or_safepoint(Threads_lock); 
>    // ALL_JAVA_THREADS iterates through all JavaThreads 
>    ALL_JAVA_THREADS(p) { 
>      tc->do_thread(p); 
>    } 
>
> .... 
>
>
> -Zhengyu 
>
>
>
>
> > Roman 
> > 
> >> 
> >> Thanks, 
> >> 
> >> -Zhengyu 
> >> 
> >> On 12/05/2016 11:00 AM, Aleksey Shipilev wrote: 
> >>> Hi, 
> >>> 
> >>> Currently, when concurrent GC is canceled, we still enter the VM 
> >>> operation for 
> >>> concurrent evacuation, only to exit it quickly and slide into the 
> >>> full GC. This 
> >>> causes *two* back-to-back safepoints: one short from evac, and 
> >>> another large for 
> >>> full GC. While short one is normally short, it can hit the unlucky 
> >>> scheduling 
> >>> outlier and drag the pause time up. 
> >>> 
> >>> This change avoids going to evac if conc GC was canceled: 
> >>>     http://cr.openjdk.java.net/~shade/shenandoah/cancel-no-evac/webr 
> >>> ev.01/ 
> >>> 
> >>> Additionally, it resets the mark bitmaps before full GC with 
> >>> parallel workers, 
> >>> not concurrent ones, which would be important once Zhengyu trims 
> >>> down the number 
> >>> of concurrent workers. 
> >>> 
> >>> Testing: hotspot_gc_shenandoah, jcstress (tests-all/quick) 
> >>> 
> >>> Thanks, 
> >>> -Aleksey 
> >>> 
>