RFR (S) 8249938: Move mirror oops from Universe into OopStorage

[coleen.phillimore at oracle.com]

Thanks David!
Coleen

On 7/24/20 1:30 AM, David Holmes wrote:
> Hi Coleen,
>
> This all seems fine to me.
>
> Thanks,
> David
>
> On 24/07/2020 11:43 am, coleen.phillimore at oracle.com wrote:
>>
>> For the record and for reviewer #2, this is the incremental webrev, 
>> with a bug fixed in ReadClosure::do_oop.
>>
>> incr webrev at 
>> http://cr.openjdk.java.net/~coleenp/2020/8249938.02.incr/webrev
>> full webrev at 
>> http://cr.openjdk.java.net/~coleenp/2020/8249938.02/webrev
>>
>> thanks,
>> Coleen
>>
>>
>> On 7/23/20 5:49 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> Thank you for reviewing, Ioi.
>>>
>>> On 7/23/20 4:41 PM, Ioi Lam wrote:
>>>>
>>>>
>>>> On 7/23/20 10:05 AM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: Save and restore mirror oops to temporary array for CDS, 
>>>>> and move them to OopStorage once restored.
>>>>>
>>>>> This is a subtask of moving oops out of Universe.  I ran 
>>>>> performance tested of this and there is no performance change. 
>>>>> Some slight decrease in number of instructions (improvement!) in 
>>>>> Perfstartup-Noop that were flagged as significant - 0.10%
>>>>>
>>>>> Tested with mach5 tier1-3.
>>>>>
>>>>> open webrev at 
>>>>> http://cr.openjdk.java.net/~coleenp/2020/8249938.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8249938
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>
>>>> Hi Coleen,
>>>>
>>>> The changes looks good. I think Universe::serialize() can be 
>>>> simplified as (not tested):
>>>>
>>>> void Universe::serialize(SerializeClosure* f) {
>>>>
>>>> #if INCLUDE_CDS_JAVA_HEAP
>>>>   {
>>>>     oop mirror_oop;
>>>>     for (int i = T_BOOLEAN; i < T_VOID+1; i++) {
>>>>       if (f->is_reading()) {   // f->reading() ...
>>>>         f->do_oop(&mirror_oop); // read from archive
>>>>         _mirrors[i] = OopHandle(vm_global(), mirror_oop);
>>>>       } else {
>>>>         mirror_oop = _mirrors[i].resolve();
>>>>         f->do_oop(&mirror_oop); // write to archive
>>>>       }
>>>>       if (mirror_oop != NULL) { // may be null if archived heap is 
>>>> disabled
>>>> java_lang_Class::update_archived_primitive_mirror_native_pointers(mirror_oop); 
>>>>
>>>>       }
>>>>     }
>>>>   }
>>>> #endif
>>>
>>> Yes that works and is better.  I think GC hates when the location of 
>>> mirror_oop is the same, but serializing doesn't care.  I've rerun 
>>> tier1 tests on it and it all passes.
>>
>>
>>> Thanks!
>>> Coleen
>>>>
>>>> Thanks
>>>> - Ioi
>>>
>>