Request for Review (s) - 8014862:Add fast Metasapce capacity and used per MetadataType

[Jon Masamitsu]

Stefan,

Thanks for the review.

On 5/20/13 2:10 AM, Stefan Karlsson wrote:
> On 05/20/2013 07:40 AM, Jon Masamitsu wrote:
>> This fix calculates the running sums for allocated capacity and
>> allocated used in MetaspaceAux to be  per MetaspaceType instead of
>> the sum of both types.
>>
>> http://cr.openjdk.java.net/~jmasa/8014862/webrev.00/
>
> I think this split looks mostly good. I have a few comments.
>
> http://cr.openjdk.java.net/~jmasa/8014862/webrev.00/src/share/vm/memory/metaspace.cpp.frames.html 
>
>
> 2497     err_msg("About to decrement below 0: words " SIZE_FORMAT
> 2498             " is greater than _allocated_capacity_words[%u] " 
> SIZE_FORMAT,
> 2499             mdtype, words, allocated_capacity_words(mdtype)));
>
> I think the order of mdtypes and words is wrong.

Agreed.  Fixed.

>
>
> 2511     err_msg("About to decrement below 0: words " SIZE_FORMAT
> 2512             " is greater than _allocated_used_words " SIZE_FORMAT,
> 2513             words, allocated_used_words(mdtype)));
>
> This err_msg line doesn't say what mdtype is (like you do in the 
> err_msg above).
>
> 2630 // The calls to capacity_bytes_slow() and used_bytes_slow() cause
> 2631 // lock ordering assertion failures with some collectors.  Do
> 2632 // not include this code until the lock ordering is fixed.
>
> This comment should be removed.

Done.

>
>
> http://cr.openjdk.java.net/~jmasa/8014862/webrev.00/src/share/vm/memory/metaspace.hpp.patch 
>
>
> -  enum MetadataType {ClassType, NonClassType};
> +  enum MetadataType {ClassType = 0,
> +             NonClassType = ClassType + 1,
> +             ClassTypeCount = ClassType + 2
> +  };
>
> Is there a reason why you had to explicitly enumerate the values here?

I understood that compilers are allowed to assign any distinct values to
the enums.   So here I make sure they are consecutive.

>
> ClassTypeCount doesn't seem to be an appropriate name. Maybe use 
> MetadataTypeCount instead?

Using MetadataTypeCount now

Jon

>
> thanks,
> StefanK