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