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

[serguei.spitsyn at oracle.com]

Ok, thanks!
Serguei

On 3/7/13 11:34 AM, Coleen Phillimore wrote:
>
> Thank you for reviewing the code, Serguei.
>
> On 3/7/2013 2:18 PM, serguei.spitsyn at oracle.com wrote:
>> 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?
>
> It was never used by the SA and doesn't really have a use in an error 
> condition.  It's used to control redundant verifies.
>>
>> 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?
>
> These fields were moved to ConstMethod so are initialized in it's 
> constructor.
>
> Thanks!
> Coleen
>>
>> 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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>