Replace j9time_current_time_millis() with j9time_hires_clock() in gc #17739
Conversation
runtime/gc_trace/TgcCardCleaning.cpp
Outdated
| @@ -58,7 +58,7 @@ printCardCleaningStats(OMR_VMThread *omrVMThread) | |||
| U_64 totalTime = 0; | |||
| UDATA totalCardsCleaned = 0; | |||
|
|
|||
| omrstr_ftime_ex(timestamp, sizeof(timestamp), "%b %d %H:%M:%S %Y", j9time_current_time_millis(), OMRSTR_FTIME_FLAG_LOCAL); | |||
| omrstr_ftime_ex(timestamp, sizeof(timestamp), "%b %d %H:%M:%S %Y", j9time_hires_clock(), OMRSTR_FTIME_FLAG_LOCAL); | |||
There was a problem hiding this comment.
It is not the same: j9time_current_time_millis() returns time in milliseconds since January 1st 1970 UTC but j9time_hires_clock() output is read of high-resolution performance counter (ticks).
There was a problem hiding this comment.
I guess we can just leave this unchanged, and care only about those that calculate time diff?
There was a problem hiding this comment.
Yes, this is correct. We need to fix only critical ones (like cases where delta time may be calculated). No need to modify places like general day/time reporting.
kangyining
changed the title
| @@ -425,7 +425,7 @@ MM_IncrementalGenerationalGC::globalMarkPhase(MM_EnvironmentVLHGC *env, bool inc | |||
|
|
|||
| if (incrementalMark) { | |||
| reportGMPMarkStart(env); | |||
| I_64 endTime = j9time_current_time_millis() + _schedulingDelegate.currentGlobalMarkIncrementTimeMillis(env); | |||
| U_64 endTime = j9time_hires_clock() + _schedulingDelegate.currentGlobalMarkIncrementTimeMillis(env)*j9time_hires_frequency()/1000; | |||
There was a problem hiding this comment.
minor, formating: keep spaces around binary operators like '*', '/'
|
looks good. remove WIP and squash the commits |
| @@ -190,7 +190,7 @@ class MM_ParallelScrubCardTableTask : public MM_ParallelTask | |||
| private: | |||
| MM_CycleState * const _cycleState; /**< Current cycle state information */ | |||
| bool _timeLimitWasHit; /**< true if any requests to check the time came in after we had hit our time threshold */ | |||
| const I_64 _timeThreshold; /**< The millisecond which represents the end of this increment */ | |||
| const U_64 _timeThreshold; /**< The millisecond which represents the end of this increment */ | |||
There was a problem hiding this comment.
Is this variable stored ticks instead of milliseconds now? If so, comment should be changed. Here and below
There was a problem hiding this comment.
Does not seem to be fixed yet.
Note, there a few of them to fix. For example, comment above declaration markScan.
There was a problem hiding this comment.
I'm not sure why they are not there last Friday, but they should be fine now.
kangyining
force-pushed
the
issue_17706
branch
2 times, most recently
from
6799c2f
to
96765c8
Compare
kangyining
changed the title
The j9time_current_time_millis() uses UNIX gettimeofday() function, and is therefore non-monotonic and error-prone. We turn to use monotonic j9time_hires_clock() function call. Issue: eclipse-openj9#17706
|
jenkins test sanity win jdk11 |
|
jenkins compile aix jdk17 |
The j9time_current_time_millis() uses UNIX gettimeofday() function, and is therefore non-monotonic and error-prone.
We turn to use monotonic j9time_hires_clock() function call.
Issue: #17706