RFR (3*XS): Backports to 8u40 of 8054819, 8055919, 8056043

Thomas Schatzl thomas.schatzl at oracle.com
Tue Sep 2 20:57:05 UTC 2014


Hi Jon,

On Tue, 2014-09-02 at 13:33 -0700, Jon Masamitsu wrote:
> Thomas,
> 
> No correctness issues so looks good.
> 
> A few minor things.  Ignore them if they would make future backports
> less automatic.

Unfortunately it does not matter. We did not backport a few typo fixes
to 8u, so we have merge errors all over the place.

Also parts of the p2i()/PTR_FORMAT mess-up are not in 8u yet.

So there is already quite a lot of need for manual fixing :(

> 
> Could you fix the indentation (line-up the variables) here?
> http://cr.openjdk.java.net/~tschatzl/8054819/webrev.8u40/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.udiff.html
> 
>   class RebuildRegionSetsClosure : public HeapRegionClosure {
>   private:
>     bool            _free_list_only;
>     HeapRegionSet*   _old_set;
> -  HeapRegionSeq*   _hrs;
> +  HeapRegionManager*   _hrm;
>     size_t          _total_used;
> 
> also
> 
>   class VerifyRegionListsClosure : public HeapRegionClosure {
>   private:
>     HeapRegionSet*   _old_set;
>     HeapRegionSet*   _humongous_set;
> -  HeapRegionSeq*   _hrs;
> +  HeapRegionManager*   _hrm;

I will look at this. Spacing seems to be correct here (at least in the
jdk9 tree and the diff), but at the moment I do not have access to my
8u-tree with that change applied.

If there is, I will fix it in the change mentioned below.

> http://cr.openjdk.java.net/~tschatzl/8054819/webrev.8u40/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp.udiff.html
> 
> 
> -  RegionIdx_t from_hrs_ind = (RegionIdx_t) from_hr->hrs_index();
> +  RegionIdx_t from_hrs_ind = (RegionIdx_t) from_hr->hrm_index();
>   
> Variable name to from_hrm_ind ?
> 
> which would flow into this change
> 
> -                        "overflow(f: %d, t: %d)",
> -                        tid, from_hrs_ind, cur_hrs_ind);
> +                        "overflow(f: %d, t: %u)",
> +                        tid, from_hrs_ind, cur_hrm_ind);
> 
> from_hrs_ind ?

I already noticed that. It's not in the original change too, so I did
not change that.

I will fix at least the latter in a separate change.

Thanks a lot,
  Thomas





More information about the hotspot-gc-dev mailing list