RFR: Properly update roots in code cache

[Roman Kennke]

Am 29.06.2017 um 08:11 schrieb Aleksey Shipilev:
> On 06/28/2017 11:01 PM, Roman Kennke wrote:
>> With concurrent code cache evacuation, we now have to make sure we
>> properly update refs in the code cache at the start of full-gc. The
>> SCM::update_roots() only asserts that code cache roots point to
>> to-space, which is not correct under conc codecache evac. This patch
>> changes the code so that it properly updates code cache roots in full GC:
>>
>> http://cr.openjdk.java.net/~rkennke/fixfullupdatecodecache/webrev.00/
>> <http://cr.openjdk.java.net/%7Erkennke/fixfullupdatecodecache/webrev.00/>
> Ah. Makes sense.
>
> But the patch looks fragile for two reasons:
>
>  a) The change is within the ASSERT block, meaning release is still broken
>  b) It relies on cancelled_concgc to detect we are in full GC path, which might
> get accidentally reset when we are in full GC already
>
> I would rather split ShenandoahConcurrentMark::update_roots to normal and Full
> GC versions, and act accordingly. Or maybe keep it single, but accept the
> boolean flag, e.g.:
>
> void ShenandoahConcurrentMark::update_roots(bool full_gc) {
>   ShenandoahGCPhase phase(full_gc ?
>                           ShenandoahCollectorPolicy::full_gc_roots :
>                           ShenandoahCollectorPolicy::final_update_refs_roots);
>   ...
>   if (full_gc) {
>     ...do unconditional root update...
>   } else {
>     ...do what we have now...
>   }
> }
Like this:
http://cr.openjdk.java.net/~rkennke/fixfullupdatecodecache/webrev.01/
<http://cr.openjdk.java.net/%7Erkennke/fixfullupdatecodecache/webrev.01/>

?