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

[serguei.spitsyn at oracle.com]

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* 
>>> loader_data,  int length, TRAPS) {*
>>> *+                                      int length,*
>>> *+                                      const intStack& index_map,*
>>> *+                                      const intStack& 
>>> invokedynamic_map, 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           _verify_count;  // 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/view_bug.do?bug_id=8003553
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>
>>>
>>
>