RFR (L): 8073013: Add detailed information about PLAB memory usage
Erik Helin
erik.helin at oracle.com
Mon Aug 17 09:00:35 UTC 2015
On 2015-08-13, Thomas Schatzl wrote:
> Hi,
>
> On Wed, 2015-08-12 at 15:13 +0200, Mikael Gerdin wrote:
> > Hi Thomas,
> >
> > On 2015-08-12 10:44, 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)
> >
> > I only had a cursory look at the change, overall it seems good and I
> > know there's a large patch series to manage here.
> > However, in plab.hpp you've added includes of allocation.inline.hpp and
> > atomic.inline.hpp. In general we should always avoid including inline
> > headers in normal headers, it's not obvious to me why your other changes
> > to plab.hpp would require the include change.
>
> I moved out these changes into an extra CR, this is a fix for an
> existing issue acually, PLABStats referencing members of Atomic only in
> atomic.inline.hpp.
>
> Please have a look at the RFR for JDK-8133470.
>
> Following Erik's suggestion I also moved out the JFR event, see the
> review for JDK-8133530.
>
> This change includes any adaptations caused by JDK-8133470.
>
> New webrev:
> http://cr.openjdk.java.net/~tschatzl/8073013/webrev.3 (full)
Looks good, Reviewed.
Thanks,
Erik
> http://cr.openjdk.java.net/~tschatzl/8073013/webrev.2_to_3 (diff)
>
> Testing:
> jprt
>
> Thanks,
> Thomas
>
>
>
More information about the hotspot-gc-dev
mailing list