RFR (S): 8143215: gcc 4.1.2: fix three issues breaking the build.

[Lindenmaier, Goetz]

Hi,

I would call for an extra change if you want to change this.
I think it's strange a flag specifying a time span depends on 
the platform in it's presicion (intx).  So I think the type of the 
Flag should be changed to uint64_t or the like.  This would be 
out of the scope of a 'fix the build' change.

Best regards,
  Goetz.

> -----Original Message-----
> From: Kim Barrett [mailto:kim.barrett at oracle.com]
> Sent: Sonntag, 13. Dezember 2015 00:03
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: hotspot-gc-dev at openjdk.java.net
> Subject: Re: RFR (S): 8143215: gcc 4.1.2: fix three issues breaking the build.
> 
> On Dec 11, 2015, at 5:52 AM, Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com> wrote:
> >
> > diff -r 73e267ddd93d
> src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp
> > --- a/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp     Wed
> Nov 18 11:31:59 2015 +0100
> > +++ b/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp     Fri
> Dec 11 11:42:52 2015 +0100
> > @@ -2731,7 +2731,7 @@
> >   // Not MT-safe; so do not pass around these StackObj's
> >   // where they may be accessed by other threads.
> >   jlong wallclock_millis() {
> > -    return TimeHelper::counter_to_millis(os::elapsed_counter() -
> _trace_time.start_time());
> > +    return (jlong)TimeHelper::counter_to_millis(os::elapsed_counter() -
> _trace_time.start_time());
> >   }
> > };
> 
> This conversion was introduced by the recently pushed 8145092: Use Unified
> Logging for the GC logging.
> 
> The only caller of wallclock_millis compares the result with
> CMSMaxAbortablePrecleanTime (an intx).
> 
> So I think we've always had a bit of a type mismatch here.
> 
> I'm not sure what to do about this one yet, but adding a cast doesn't seem
> like the right solution to me.
> 
> > diff -r 73e267ddd93d src/share/vm/gc/g1/g1CollectedHeap.cpp
> > --- a/src/share/vm/gc/g1/g1CollectedHeap.cpp    Wed Nov 18 11:31:59 2015
> +0100
> > +++ b/src/share/vm/gc/g1/g1CollectedHeap.cpp    Fri Dec 11 11:42:52 2015
> +0100
> > @@ -3621,7 +3621,7 @@
> >     log_info(gc)("To-space exhausted");
> >   }
> >
> > -  double pause_time_sec =
> TimeHelper::counter_to_seconds(pause_time_counter);
> > +  double pause_time_sec =
> TimeHelper::counter_to_seconds((jlong)pause_time_counter);
> >   g1_policy()->print_phases(pause_time_sec);
> >
> >   g1_policy()->print_detailed_heap_transition();
> 
> Adding a cast is not the proper fix here.
> 
> The problem is that log_gc_footer's double pause_time_counter argument
> ought to itself be a jlong.
> The caller (do_collection_pause_at_safepoint) contains
> 
>   double pause_start_counter = os::elapsed_counter();
>   ...
>   log_gc_footer(os::elapsed_counter() - pause_start_counter);
> 
> elapsed_counter returns a jlong.  So pause_start_counter ought to be a jlong
> too, and fixing the
> parameter type for log_gc_footer would then make everything work out.
> 
> This appears to be a mistake introduced by the recently pushed 8145092: Use
> Unified Logging for the GC logging.