RFR: Purge unnecessary time conversion in ShenandoahPhaseTimings::record_phase_time

Zhengyu Gu zgu at redhat.com
Thu Nov 1 16:52:56 UTC 2018


Good.

-Zhengyu

On 11/01/2018 12:49 PM, Aleksey Shipilev wrote:
> This was found when building sh/jdk8u on Mac OS X: implicit double -> jint conversion makes clang
> unhappy. In fact, we don't even need to do this conversion, because we can just pass the double
> along, and this also saves us from accidental truncation.
> 
> The fix goes to sh/jdk and then proliferates with backports:
> 
> diff -r c4640bf7aeb3 src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp     Thu Nov 01 17:16:47 2018 +0100
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp     Thu Nov 01 17:19:32 2018 +0100
> @@ -62,10 +62,10 @@
>     ShenandoahHeap::heap()->heuristics()->record_phase_time(phase, elapsed);
>   }
> 
> -void ShenandoahPhaseTimings::record_phase_time(Phase phase, jint time_us) {
> +void ShenandoahPhaseTimings::record_phase_time(Phase phase, double time) {
>     assert(_policy != NULL, "Not yet initialized");
>     if (!_policy->is_at_shutdown()) {
> -    _timing_data[phase]._secs.add((double)time_us / 1000 / 1000);
> +    _timing_data[phase]._secs.add(time);
>     }
>   }
> 
> diff -r c4640bf7aeb3 src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp     Thu Nov 01 17:16:47 2018 +0100
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp     Thu Nov 01 17:19:32 2018 +0100
> @@ -347,8 +347,8 @@
>     void record_phase_start(Phase phase);
>     // record phase end and return elapsed time in seconds for the phase
>     void record_phase_end(Phase phase);
> -  // record an elapsed time in microseconds for the phase
> -  void record_phase_time(Phase phase, jint time_us);
> +  // record an elapsed time for the phase
> +  void record_phase_time(Phase phase, double time);
> 
>     void record_workers_start(Phase phase);
>     void record_workers_end(Phase phase);
> diff -r c4640bf7aeb3 src/hotspot/share/gc/shenandoah/shenandoahTimingTracker.cpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahTimingTracker.cpp     Thu Nov 01 17:16:47 2018 +0100
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahTimingTracker.cpp     Thu Nov 01 17:19:32 2018 +0100
> @@ -87,6 +87,6 @@
>     ShenandoahPhaseTimings* phase_times = ShenandoahHeap::heap()->phase_timings();
> 
>     double t = phase_times->termination_times()->average();
> -  phase_times->record_phase_time(_phase, t * 1000 * 1000);
> +  phase_times->record_phase_time(_phase, t);
>     debug_only(_current_termination_phase = ShenandoahPhaseTimings::_num_phases;)
>   }
> 
> 
> Testing: sh/jdk tier3_gc_shenandoah, sh/jdk8u Mac OS X builds
> 
> Thanks,
> -Aleksey
> 


More information about the shenandoah-dev mailing list