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

[Mikael Gerdin]

Hi Thomas,

On 2015-08-13 17:32, 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)
> http://cr.openjdk.java.net/~tschatzl/8073013/webrev.2_to_3 (diff)

I think this is ready to go now, Reviewed.

/Mikael

>
> Testing:
> jprt
>
> Thanks,
>    Thomas
>
>
>