RFR: 8024718: Metaspace performance counters and memory pools should report the same data

[Erik Helin]

On 2013-09-13, David Holmes wrote:
> Hi Erik,
> Just looking at the tests ...

Hi David,
thanks for reviewing the tests!

On 2013-09-13, David Holmes wrote:
> Can you please add -XX:+UsePerfData to the @run lines of the new test.

Done.

On 2013-09-13, David Holmes wrote:
> Have you discussed the change to InputArguments.contains?

I'm the one who added the class InputArguments to testlibrary and I'm
also the only one that have been using it so far when writing tests :)

However, I have CC:d Christian and Igor on this email, if they have an
opinion on this.

On 2013-09-13, David Holmes wrote:
> If we have any args that are substrings of other args then this change
> will cause incorrect matching.

I actually do not know if any flag is a substring of any other flag, but
you are right that it could potentially be confusing.

On 2013-09-13, David Holmes wrote:
> Would it not be better to add a new function that checks the prefix
> rather than the whole arg?

Agree. I suggest the following two methods:
- InputArguments.contains, works the same way as today
- InputArguments.containsPrefix, checks if one of the arguments contains
  the given prefix

I've uploaded a new webrev at:
http://cr.openjdk.java.net/~ehelin/8024718/webrev.01/

What do you think?

Thanks,
Erik

> Thanks,
> David
> 
> On 13/09/2013 7:46 AM, Erik Helin wrote:
> >Hi all,
> >
> >this patch fixes some issues with memory pools and the performance
> >counter reporting slightly different numbers for metaspace. I've also
> >updated two jtreg tests as well as added a new one to test the change.
> >
> >InputArguments.java in the testlibrary is updated as well.
> >InputArguments.contains now takes a prefix as a parameter instead of the
> >exact name of a flag. This makes it possible to check if a flag expecting
> >a value has been set. For example, -XX:MaxMetaspaceSize=100m can be
> >checked by running InputArguments.contains("-XX:MaxMetaspaceSize").
> >
> >Webrev:
> >http://cr.openjdk.java.net/~ehelin/8024718/webrev.00/
> >
> >Testing:
> >- JPRT
> >- hotspot/test/gc (including updated and new tests)
> >
> >Thanks,
> >Erik
> >