Review request (S) 8003553: NPG: metaspace objects should be zeroed in constructors (original) (raw)

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Thu Mar 7 11🔞24 PST 2013

Coleen,

It is good, but a couple of questions below.

Q1: The field " _verify_count" is moved from the Klass to the InstanceKlass. But it is removed from the vmStructs.cpp. Is it because the field is NOT_PRODUCT or it is just useless for the SA?

Q2: The following initializations are removed from the method.cpp:Method::Method() :

86 set_name_index(0); 87 set_signature_index(0); . . . 91 set_constants(NULL); 92 set_max_stack(0); 93 set_max_locals(0);

 It looks like a move in reverse direction.
 Are those dups?

Thanks, Serguei

On 3/7/13 4:59 AM, Coleen Phillimore wrote:

I need another reviewer for this (and two for the other change). This one is really easy. You don't need to be a Reviewer. Coleen On 3/6/2013 4:25 PM, Coleen Phillimore wrote:

Jon, Thank you for reviewing this change. On 03/06/2013 12:26 AM, Jon Masamitsu wrote: http://cr.openjdk.java.net/~coleenp/8003553/src/share/vm/oops/cpCache.cpp.udiff.html

- ConstantPoolCache ConstantPoolCache::allocate(ClassLoaderData* loaderdata, int length, TRAPS) {* + int length, + const intStack& indexmap, *+ const intStack& invokedynamicmap, TRAPS) { * Why did you not move the TRAP parameter to a new line? I think TRAPS aren't interesting enough and short enough not to merit their own line. http://cr.openjdk.java.net/~coleenp/8003553/src/share/vm/oops/methodData.cpp.udiff.html Did you remove the TieredCompilation test *if (TieredCompilation) { * for consistency of initialization? Yes, I did. I don't know if people will object but I think it's more sanitary to have these zero initialized even if they are not used in !TieredCompilation. And I had a consistency check that I took out that flagged these.

http://cr.openjdk.java.net/~coleenp/8003553/src/share/vm/oops/klass.hpp.udiff.html * #ifndef PRODUCT* * int verifycount; // to avoid redundant verifies* * #endif* Not concerned about redundant verifies anymore? I moved it from Klass* to InstanceKlass* where it's used. It still protects us from redundant verifies. Rest looks good. Thanks! Coleen Jon On 3/5/2013 11:30 AM, Coleen Phillimore wrote: Adding hotspot-dev to get some more potential reviewers from maybe the GC team, hint... Coleen On 03/04/2013 03:02 PM, Coleen Phillimore wrote: Summary: Zero metadata in constructors, not in allocation (and some in constructors) This seems like a good first step in passing initial values into constructors and no initializing metadata types by the callers (although a lot more parameters will have to be passed for some). Tested with runThese jck, NSK vm.quick.testlist, lang and vm jck8, java/lang/annotation jtreg tests and java/lang/invoke jtreg tests. open webrev at http://cr.openjdk.java.net/~coleenp/8003553/ bug link at http://bugs.sun.com/viewbug.do?bugid=8003553 Thanks, Coleen

More information about the hotspot-dev mailing list