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

[Lindenmaier, Goetz]

Hi,

I removed the change to g1CollectedHeap.cpp    
("8145303: Clean up the units for log_gc_footer" will fix this)
and changed the flag to 'double'. New webrev:
http://cr.openjdk.java.net/~goetz/webrevs/8143215-gcc412/webrev.05/

Best regards,
  Goetz.


> -----Original Message-----
> From: Bengt Rutisson [mailto:bengt.rutisson at oracle.com]
> Sent: Montag, 14. Dezember 2015 10:44
> To: Kim Barrett <kim.barrett at oracle.com>; 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.
> 
> 
> Hi Kim and Goetz,
> 
> On 2015-12-13 00:02, Kim Barrett wrote:
> > 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.
> 
> I would suggest changing CMSMaxAbortablePrecleanTime to a double. It
> may
> not be the cleanest way to handle the flag but having it as an intx is
> also not very clean. Negative values won't make sense. Changing it to a
> double won't make the situation much worse than it is now and it will
> simplify the code.
> 
> So, I would suggest:
> 
> diff --git a/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp
> b/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp
> --- a/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp
> +++ b/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp
> @@ -2733,1 +2733,1 @@
> -  jlong wallclock_millis() {
> +  double wallclock_millis() {
> diff --git a/src/share/vm/runtime/globals.hpp
> b/src/share/vm/runtime/globals.hpp
> --- a/src/share/vm/runtime/globals.hpp
> +++ b/src/share/vm/runtime/globals.hpp
> @@ -1836,1 +1836,1 @@
> -  product(intx, CMSMaxAbortablePrecleanTime,
> 5000,                          \
> +  product(double, CMSMaxAbortablePrecleanTime,
> 5000,                        \
> 
> 
> >
> >> 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.
> >
> 
> Yes, this is a mistake in the push for the unified logging work. I
> created a patch and sent out a new review request to fix it.
> 
> RFR: JDK-8145303: Clean up the units for log_gc_footer
> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-
> December/015903.html
> 
> http://cr.openjdk.java.net/~brutisso/8145303/webrev.00/
> 
> Thanks,
> Bengt