RFR: 8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp

[David Holmes]

On 27/01/2021 5:52 pm, Ioi Lam wrote:
> On Wed, 27 Jan 2021 06:27:51 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> The "well-known classes" API in [systemDictionary.hpp](https://github.com/openjdk/jdk/blob/19b6f61bd2f49f06ef6b8e9b6ecd2fd910070035/src/hotspot/share/classfile/systemDictionary.hpp#L103) is for accessing a fixed set of classes defined by the bootstrap loader. The rest of systemDictionary.hpp is for finding/defining arbitrary classes in arbitrary class loaders. For modularity, we should separate these two sets of APIs, and make them independent of each other.
>>>
>>> The proposed API follows the existing pattern of accessing known symbols/intrinsics in [vmSymbols.hpp](https://github.com/openjdk/jdk/blob/19b6f61bd2f49f06ef6b8e9b6ecd2fd910070035/src/hotspot/share/classfile/vmSymbols.hpp#L53) and [vmIntrinsics.hpp](https://github.com/openjdk/jdk/blob/19b6f61bd2f49f06ef6b8e9b6ecd2fd910070035/src/hotspot/share/classfile/vmIntrinsics.hpp#L108):
>>>
>>> #include "classfile/vmClasses.hpp"
>>> #include "classfile/vmIntrinsics.hpp"
>>> #include "classfile/vmSymbols.hpp" // NEW
>>>
>>> Symbol* s = vmSymbols::java_lang_Object();
>>> vmIntrinsics::ID id = vmIntrinsics::_linkToSpecial;
>>> Klass* k = vmClasses::Object_klass(); // NEW
>>>
>>> Note: to help the review process and limit the amount of boiler-place changes, within this RFE, we do not change existing calls like `SystemDictionary::Object_klass()` to `vmClasses::Object_klass()`. That will be done in a subtask ([JDK-8260471](https://bugs.openjdk.java.net/browse/JDK-8260471) -- [preliminary webrev](http://cr.openjdk.java.net/~iklam/jdk17/8260471-decouple-vmClasses-and-SystemDictionary/)).
>>>
>>> Within this RFE, we temporarily subclass `SystemDictionary` from `vmClasses`, so `SystemDictionary::Object_klass()` will continue to work.
>>>
>>> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>>
>> src/hotspot/share/classfile/vmClasses.cpp line 219:
>>
>>> 217: #if INCLUDE_CDS
>>> 218:
>>> 219: void vmClasses::resolve_shared_class(InstanceKlass* klass, ClassLoaderData* loader_data, Handle domain, TRAPS) {
>>
>> It isn't at all obvious to me why this functionality was relocated. I don't see anything vmClasses related in here. ??
> 
> `vmClasses::resolve_shared_class()` is a private method and is called from here:
> 
> bool vmClasses::resolve(VMClassID id, TRAPS) {
>    InstanceKlass** klassp = &_klasses[as_int(id)];
> ...
>      InstanceKlass* k = *klassp;
>      resolve_shared_class(k, ....);  <---- HERE
> }
> 
> Maybe the function name is not clear enough. It's meant to "resolve a vmClass from the CDS archive". It used to be the (even more confusingly named) `SystemDictionary::quick_resolve()` method.

Thanks for clarifying - the fact it has a single caller was not clear.

David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/2246
>