RFR (L): 8073013: Add detailed information about PLAB memory usage

Thomas Schatzl thomas.schatzl at oracle.com
Thu Aug 13 10:54:38 UTC 2015


Hi Erik,

On Thu, 2015-08-13 at 12:33 +0200, Erik Helin wrote:
> Hi Thomas,
> 
> On 2015-08-12, Thomas Schatzl wrote:
> > Hi David,
> > 
> >   thanks for the review:
> > 
> > On Tue, 2015-08-11 at 15:07 +0200, David Lindholm wrote:
> > > Hi Thomas.
> > > 
> > > This looks good, except for the changes in trace.xml. All fields should 
> > > follow Java naming conventions, for example regionEndWaste, not 
> > > region_end_waste. Also, 2 different event types are preferred according 
> > > to the Servicability folks (instead of the "gen" field). Suggested names 
> > > are GCG1EvacuationYoungStatistics and GCG1EvacuationOldStatistics.
> > > 
> > > The other parts looks good.
> > 
> > I think all fixed in the new webrevs at:
> > 
> > http://cr.openjdk.java.net/~tschatzl/8073013/webrev.2 (full)
> > http://cr.openjdk.java.net/~tschatzl/8073013/webrev.1_to_2 (diff)
> 
> just two comments:
> - could you split out the trace event into another patch to make this
>   patch smaller?
> - I have not reviewed the algorithm for
>   G1EvacStats::adjust_desired_plab_sz in any more depth than veryfing
>   that the copy from plab.cpp seems correct (I also know that there are
>   more patches coming with changes to this logic).
> 
> Other than splitting out the trace event and Mikael's comment, I think
> the patch looks good.
> 

  thanks for your review. Please look at my response to Mikael and the
new RFR for the split out JFR events.

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list