Request for review (xs) 8006040: NPG: on_stack processing wastes space in ConstantPool

Coleen Phillimore coleen.phillimore at oracle.com
Wed Jan 23 07:26:31 PST 2013


Thanks, David!
Coleen

On 01/22/2013 05:24 PM, David Holmes wrote:
> Thanks Colleen. Looks okay to me. (I saw the _2 version :))
>
> David
>
> On 23/01/2013 5:21 AM, Coleen Phillimore wrote:
>>
>> David,
>>
>> Thank you for noticing my code review!! I have an update at
>> http://cr.openjdk.java.net/~coleenp/8006040/
>> that addresses your comments.
>> I need another reviewer!
>>
>> On 01/20/2013 09:29 PM, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> constantPool.cpp:
>>>
>>> // all fields are initialized; needed for GC
>>> - set_on_stack(false);
>>
>> I don't think this comment means anything anymore. I think it had
>> something to do with the constantPoolOop object being partially
>> initialized when hitting a GC point before Permgen was eliminated. I'll
>> remove the comment also.
>>
>>>
>>> Isn't the comment tied to the now deleted line?
>>>
>>> ! if (has_pseudo_string() || has_invokedynamic() ||
>>> has_preresolution()) {
>>> if (has_pseudo_string()) st->print(" has_pseudo_string");
>>> if (has_invokedynamic()) st->print(" has_invokedynamic");
>>> if (has_preresolution()) st->print(" has_preresolution");
>>> st->cr();
>>> }
>>>
>>> The double-checking of the conditions here is not nice.
>>
>> I had this change for a previous version so that the flags could be all
>> on one line (and _flags was removed in my previous version). I restored
>> the code and added printing the on_stack flag too.
>>
>> Coleen
>>
>>>
>>> Otherwise flag changes look good, as do moving MetadataMarkOnStack
>>> segements.
>>>
>>> Thanks,
>>> David
>>>
>>> On 19/01/2013 2:41 AM, Coleen Phillimore wrote:
>>>>
>>>> This is relatively easy, anyone? Also, it doesn't affect the SA. I
>>>> checked.
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 1/16/2013 4:23 PM, Coleen Phillimore wrote:
>>>>> Summary: Added on_stack bit to _flags. Also MetadataMarkOnStack is
>>>>> used for more than JVMTI so had to be moved.
>>>>>
>>>>> Confirmed with John and Chris that setting invokedynamic bits doesn't
>>>>> require atomic operations so I can add on_stack to the flags.
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8006040/
>>>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=8006040
>>>>>
>>>>> Tested NSK quick.testlist and runThese.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>
>>



More information about the hotspot-runtime-dev mailing list