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

[Ioi Lam]

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
- 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
>>
>>