RFR (L): 8073013: Add detailed information about PLAB memory usage
Thomas Schatzl
thomas.schatzl at oracle.com
Thu Aug 13 15:32:28 UTC 2015
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)
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