RFR: Add JFR parallel and concurrent events
Zhengyu Gu
zgu at redhat.com
Thu Oct 11 13:12:04 UTC 2018
>
> Nits:
>
> *) Excess newline in shenandoahUtils.cpp:
>
> 144
>
>
> *) I think we can use switch here:
>
> 119 bool ShenandoahGCPhase::is_root_work_phase() {
> 120 ShenandoahPhaseTimings::Phase phase = current_phase();
> 121 return phase == ShenandoahPhaseTimings::scan_roots ||
> 122 phase == ShenandoahPhaseTimings::update_roots ||
> 123 phase == ShenandoahPhaseTimings::init_evac ||
> 124 phase == ShenandoahPhaseTimings::final_update_refs_roots ||
> 125 phase == ShenandoahPhaseTimings::degen_gc_update_roots ||
> 126 phase == ShenandoahPhaseTimings::init_traversal_gc_work ||
> 127 phase == ShenandoahPhaseTimings::final_traversal_gc_work ||
> 128 phase == ShenandoahPhaseTimings::final_traversal_update_roots ||
> 129 phase == ShenandoahPhaseTimings::full_gc_roots;
> 130 }
>
> *) No need for local variable "current" here?
>
> 166 ShenandoahConcurrentWorkerSession::~ShenandoahConcurrentWorkerSession() {
> 167 ShenandoahPhaseTimings::Phase current = ShenandoahGCPhase::current_phase();
> 168 _event.commit(GCId::current(), ShenandoahPhaseTimings::phase_name(current));
> 169 }
> 170
> 171 ShenandoahParallelWorkerSession::~ShenandoahParallelWorkerSession() {
> 172 ShenandoahPhaseTimings::Phase current = ShenandoahGCPhase::current_phase();
> 173 _event.commit(GCId::current(), _worker_id, ShenandoahPhaseTimings::phase_name(current));
> 174 }
>
> *) Duplicate "protected"?
>
> 121 class ShenandoahWorkerSession : public StackObj {
> 122 protected:
> 123 uint _worker_id;
> 124 protected:
> 125 ShenandoahWorkerSession(uint worker_id);
Fixed all, better?
Webrev:
http://cr.openjdk.java.net/~zgu/shenandoah/jfr_conc_par_events/webrev.01/index.html
Thanks,
-Zhengyu
>
>
> Thanks,
> -Aleksey
>
More information about the shenandoah-dev
mailing list