RFR: Properly update roots in code cache

[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...
  }
}

Thanks,
-Aleksey