RFR: 8259214: MetaspaceClosure support for Arrays of MetaspaceObj [v4]

Calvin Cheung ccheung at openjdk.java.net
Fri Jan 22 04:39:37 UTC 2021


On Tue, 19 Jan 2021 20:13:10 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Currently, `MetaspaceClosure::push` supports only the following variants:
>> 
>> MetaspaceClosure* it = ...;
>> Klass* o = ...; it->push(&o);
>> Array<int>* a1 = ...; it->push(&a1);
>> Array<Annotation*>* a2 = ...; it->push(&a2);
>> 
>> In Valhalla, support is needed for the following variant (Annotation is a subtype of MetaspaceObj):
>> 
>> Array<Annotation>* a3 = ...; it->push(&a3);
>> 
>> This change will allow CDS to make a copy of this array, as well as relocating all the pointers embedded in the elements of this array. See a test case in test_metaspaceClosure.cpp.
>> 
>> I also cleaned up the code (with help from @kimbarrett) to use SFINAE to dispatch from `MetaspaceClosure::push` to the different subtypes of `MetaspaceClosure::Ref`.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove RefMatcher and go back to overloaded template functions for push(...)

Looks good. I spotted some typos in metaspaceClosure.hpp.

src/hotspot/share/memory/metaspaceClosure.hpp line 209:

> 207: 
> 208:   // OtherArrayRef -- iterate an instance of Array<T>, where T is NOT a subtype of MetaspaceObj.
> 209:   // T can be a primitive type, since as int, or a structure. However, we do not scan

typo: s/since/such

src/hotspot/share/memory/metaspaceClosure.hpp line 321:

> 319:   // Klass*                 o  = ...;  it->push(&o);     => MSORef
> 320:   // Array<int>*            a1 = ...;  it->push(&a1);    => OtherArrayRef
> 321:   // Array<Annotation>*     a2 = ...;  it->push(&a3);    => MSOArrayRef

&a3 should be &a2?

src/hotspot/share/memory/metaspaceClosure.hpp line 322:

> 320:   // Array<int>*            a1 = ...;  it->push(&a1);    => OtherArrayRef
> 321:   // Array<Annotation>*     a2 = ...;  it->push(&a3);    => MSOArrayRef
> 322:   // Array<Klass*>*         a3 = ...;  it->push(&a2);    => MSOPointerArrayRef

&a2 should be &a3?

src/hotspot/share/memory/metaspaceClosure.hpp line 323:

> 321:   // Array<Annotation>*     a2 = ...;  it->push(&a3);    => MSOArrayRef
> 322:   // Array<Klass*>*         a3 = ...;  it->push(&a2);    => MSOPointerArrayRef
> 323:   // Array<Array<Klass*>*>* a4 = ...;  it->push(&a3);    => MSOPointerArrayRef

&a3 should be &a4?

src/hotspot/share/memory/metaspaceClosure.hpp line 324:

> 322:   // Array<Klass*>*         a3 = ...;  it->push(&a2);    => MSOPointerArrayRef
> 323:   // Array<Array<Klass*>*>* a4 = ...;  it->push(&a3);    => MSOPointerArrayRef
> 324:   // Array<Annotation>*     a5 = ...;  it->push(&a3);    => MSOPointerArrayRef

Array<Annotation>* should be Array<Annotation*>* ?

&a3 should be &a5?

-------------

Marked as reviewed by ccheung (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1995


More information about the hotspot-dev mailing list