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

David Holmes david.holmes at oracle.com
Tue Jan 22 14:24:03 PST 2013


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