RFR (T) 8250519: [REDO] Move mirror oops from Universe into OopStorage

[Ioi Lam]

On 7/24/20 7:22 PM, David Holmes wrote:
> Hi Ioi,
>
> On 25/07/2020 9:59 am, Ioi Lam wrote:
>>
>>
>> On 7/24/20 4:52 PM, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> On 25/07/2020 7:07 am, coleen.phillimore at oracle.com wrote:
>>>> Summary: The original patch but add a null pointer check so an 
>>>> OopHandle is not created if a NULL is read from the archive.
>>>>
>>>> If a NULL is read from the archive, we shouldn't create an 
>>>> OopHandle for it because one will be created in 
>>>> initialize_basic_type_mirrors. The assert added in the previous 
>>>> patch I pushed detected that.
>>>
>>> Your fix to skip NULL seems reasonable but I'm left confused as to 
>>> when and why we will find NULL for these basic type mirrors. Is it 
>>> only the reference types that will be NULL?
>>>
>>
>> Hi David,
>>
>> The null skipping happens here:
>>
>>   263       if (f->reading()) {
>>   264         f->do_oop(&mirror_oop); // read from archive
>>   265         assert(oopDesc::is_oop_or_null(mirror_oop), "is oop");
>>   266         // Only create an OopHandle for non-null mirrors
>>   267         if (mirror_oop != NULL) {
>>   268           _mirrors[i] = OopHandle(vm_global(), mirror_oop);
>>   269         }
>>
>> where the f->do_oop() will read an archived oop from the CDS heap. 
>> There are cases where the a NULL is returned inside mirror_oop:
>>
>> (1) The CDS image was created WITHOUT an archived heap (this could 
>> happen when you run -Xshare:dump with a GC or compressed oop encoding 
>> that's not compatible with the archived heap).
>>
>> (2) The CDS image has an archived heap, but the current VM is using a 
>> GC or compressed oop encoding that's not compatible with the archived 
>> heap.
>
> Thanks for the explanation. I'm having trouble seeing the equivalence 
> of the old Universe::serialize code and the new version. The new 
> version seems to perform both writing and reading - which seem odd for 
> something called "serialize". ??
>

CDS "serialize" is used for both read and writing. It's probably a bad 
name but has been there since the beginning of CDS ....

Thanks
- Ioi

> Thanks,
> David
> -----
>
>> Thanks
>> - Ioi
>>
>>
>>
>>> Thanks,
>>> David
>>>
>>>> incremental change to original change: 
>>>> http://cr.openjdk.java.net/~coleenp/2020/8250519.01.incr/webrev
>>>> full webrev at 
>>>> http://cr.openjdk.java.net/~coleenp/2020/8250519.01/webrev
>>>>
>>>> Retested with tier1-3.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>
>>