Request for review (M): 7178361: G1: Make sure that PrintGC and PrintGCDetails use the same timing for the GC pause

[John Cuthbertson]

Hi Bengt,

Thanks for taking some of my suggestions on board. I'll answer your 
question first and respond to the comments in a separate mail.

On 06/29/12 05:37, Bengt Rutisson wrote:
>
>>
>> g1RemSet.cpp
>> ------------
>> To make this consistent - perhaps change this code to call 
>> os::elapsedTime() again (rather than in 
>> G1GCPhaseTimes::record_cc_clear_time()) and pass the elapsed time delta.
>
> I don't really understand this comment. Can you explain a bit more 
> what you mean?
>

Sorry - my bad!  I meant concurrentG1Refine.cpp. I meant something like 
the following:

 503 void ConcurrentG1Refine::clear_and_record_card_counts() {
 504   if (G1ConcRSLogCacheSize == 0) {
 505     return;
 506   }
 507 
 508   double start = os::elapsedTime();
 509 
 510   if (_expand_card_counts) {
 511     int new_idx = _cache_size_index + 1;
 512 
 513     if (expand_card_count_cache(new_idx)) {
 514       // Allocation was successful and  _n_card_counts has
 515       // been updated to the new size. We only need to clear
 516       // the epochs so we don't read a bogus epoch value
 517       // when inserting a card into the hot card cache.
 518       Copy::fill_to_bytes(&_card_epochs[0], _n_card_counts * sizeof(CardEpochCacheEntry));
 519     }
 520     _expand_card_counts = false;
 521   }
 522 
 523   int this_epoch = (int) _n_periods;
 524   assert((this_epoch+1) <= max_jint, "to many periods");
 525   // Update epoch
 526   _n_periods++;
 527   double cc_clear_time_ms = (os::elapsedTime() - start) * 1000;
 528   _g1h->g1_policy()->phase_times()->record_cc_clear_time(cc_clear_time_ms);
 529 }

and:

 359 void G1GCPhaseTimes::record_cc_clear_time_ms(double ms) {
 360   if (!G1Log::finest()) {
 361     return;
 362   }
 364 
 365   if (_min_clear_cc_time_ms < 0.0 || ms <= _min_clear_cc_time_ms) {
 366     _min_clear_cc_time_ms = ms;
       }
 367   if (_max_clear_cc_time_ms < 0.0 || ms >= _max_clear_cc_time_ms) {
 368     _max_clear_cc_time_ms = ms;
       }
 369   _cur_clear_cc_time_ms = ms;
 370   _cum_clear_cc_time_ms += ms;
 371   _num_cc_clears++;
 372 }

g1RemSet.cpp looks fine.

Apologies,

JohnC

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20120702/e8de6213/attachment.htm>