Request for Review (s) - 8014862:Add fast Metasapce capacity and used per MetadataType
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/
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.
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/meta... 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. 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. http://cr.openjdk.java.net/~jmasa/8014862/webrev.00/src/share/vm/memory/meta... - 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? ClassTypeCount doesn't seem to be an appropriate name. Maybe use MetadataTypeCount instead? thanks, StefanK
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.
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/meta...
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/meta...
- 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
On 20 maj 2013, at 17:56, Jon Masamitsu <jon.masamitsu@oracle.com> wrote:
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.
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/meta...
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/meta...
- 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.
I doubt this is true. If this is the case then we have a lot of potential bugs in the code. StefanK
ClassTypeCount doesn't seem to be an appropriate name. Maybe use MetadataTypeCount instead?
Using MetadataTypeCount now
Jon
thanks, StefanK
I've updated the repository for review comments (thanks Erik and Stefan). New webrev is http://cr.openjdk.java.net/~jmasa/8014862/webrev.01/ To check the format of the warning for assert on the decrement the of used and capacity, added (temporary) debug code to print out the message of the warning.
About to decrement below 0: words 8704 is greater than _allocated_capacity_words[1] 2052864 About to decrement below 0: words 5082 is greater than _allocated_used_words[1] 2017706 About to decrement below 0: words 16 is greater than _allocated_used_words[1] 2012624 About to decrement below 0: words 768 is greater than _allocated_capacity_words[0] 248576 About to decrement below 0: words 436 is greater than _allocated_used_words[0] 219588 About to decrement below 0: words 24 is greater than _allocated_used_words[0] 219152
Jon On 5/19/13 10:40 PM, 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.
On 20 maj 2013, at 19:07, Jon Masamitsu <jon.masamitsu@oracle.com> wrote:
I've updated the repository for review comments (thanks Erik and Stefan). New webrev is
The new changes look good. StefanK
To check the format of the warning for assert on the decrement the of used and capacity, added (temporary) debug code to print out the message of the warning.
About to decrement below 0: words 8704 is greater than _allocated_capacity_words[1] 2052864 About to decrement below 0: words 5082 is greater than _allocated_used_words[1] 2017706 About to decrement below 0: words 16 is greater than _allocated_used_words[1] 2012624 About to decrement below 0: words 768 is greater than _allocated_capacity_words[0] 248576 About to decrement below 0: words 436 is greater than _allocated_used_words[0] 219588 About to decrement below 0: words 24 is greater than _allocated_used_words[0] 219152
Jon
On 5/19/13 10:40 PM, 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.
participants (2)
-
Jon Masamitsu
-
Stefan Karlsson