Shenandoah Events

[Aleksey Shipilev]

On 1/18/19 10:02 PM, Ken Dobson wrote:
> Actually I've just gotten my authorship so here is a link to the webrev.
> http://cr.openjdk.java.net/~kdobson/serializer/webrev/

Aha. How did you test it? Because the quick test fails with lots of asserts:
 $ CONF=linux-x86_64-server-fastdebug make images run-test TEST=tier1_gc_shenandoah

The patch is better to pass more complete test:
 $ CONF=linux-x86_64-server-fastdebug make images run-test TEST=hotspot_gc_shenandoah

Other comments:

 *) Copyright years need to be updated.

 *) You need to register serializers once, not for each region?

 *) It makes sense to put jfr_register_serializers into Shenandoah namespace

 *) There is no need for separate method for JFR event commit, the whole point of set_state was to
accept the JFR event inside

 *) No need for "// INCLUDE_SHENANDOAHGC" when if is short.

 *) Code style: "if(evt.should_commit())" should have space after "if".

 *) Do not change methods visibility freely. If you want to hack into private members of
ShenandoahHeapRegion, befriend the classes.

You can fix it yourself (good exercise anyway), or apply this patch on top:
 http://cr.openjdk.java.net/~shade/shenandoah/jfr-review-1.patch

-Aleksey