RFR(S) 8189140 - SystemDictionaryShared::initialize() should be renamed to be more meaningful
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Wed May 16 00:05:15 UTC 2018
Hi, I had another question/comment that I forgot to ask:
http://cr.openjdk.java.net/~iklam/jdk11/8189140-rename-system-dict-shared-initialize.v02/src/hotspot/share/classfile/systemDictionaryShared.cpp.udiff.html
SharedDictionary_lock = new Mutex(Mutex::leaf, "SharedDictionary_lock", true);
We want to put all the global locks in mutexLocker.cpp so that we can
better see their relative ranking. Can you put this there? I don't
think it's necessary to make it CDS_ONLY() really, it doesn't take an
interesting amount of space.
Or was there a reason this was here that I don't see?
thanks,
Coleen
On 5/15/18 5:47 PM, Ioi Lam wrote:
> I've updated the webrev:
>
> http://cr.openjdk.java.net/~iklam/jdk11/8189140-rename-system-dict-shared-initialize.v02/
>
>
> 1. Added JavaCalls::new_instance so we can avoid all the boiler plate
> code for allocating
> the instance andinvoking the constructor.
>
> JavaCalls::new_instance calls InstanceKlass->initialize. This is just
> a quick op after
> the class is already initialized. Also, JavaCalls::call_static also
> internally call
> into InstanceKlass->initialize, so I am just following the existing
> pattern as Coleen
> mentioned below.
>
> Doing the initialization on demand also means for cases where JAR
> manifest is not used
> (all code is loaded from the system image or directories), we get
> faster start-up.
>
> 2. I also took the time to removed a bunch of "// One oop argument"
> comments which
> probably meant something to the person who wrote it, but seems
> useless to everyone
> else.
>
> 3. As Calvin suggested, I removed the File_klass and also
> ParseUtil_klass from
> the system dictionary since they are not used anywhere. This
> hopefully improves start-up
> by a little bit, since these 2 classes are no longer resolved at
> start-up.
>
>
> (BTW, I changed the RFR subject line from XS to S due to the extend of
> change :-)
>
> Thanks
> - Ioi
>
>
>
>
> On 5/15/18 2:00 PM, coleen.phillimore at oracle.com wrote:
>>
>> http://cr.openjdk.java.net/~iklam/jdk11/8189140-rename-system-dict-shared-initialize.v01/src/hotspot/share/classfile/systemDictionaryShared.cpp.udiff.html
>>
>>
>> This looks good. This is a pattern that's used in other places, and
>> it would be better to not initialize these at startup in thread.cpp.
>>
>> Coleen
>>
>> On 5/15/18 2:07 AM, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8189140
>>> http://cr.openjdk.java.net/~iklam/jdk11/8189140-rename-system-dict-shared-initialize.v01/
>>>
>>>
>>> Summary:
>>>
>>> 1. Removed the forced initialization of a few classes used by AppCDS
>>> at JVM start-up.
>>> Instead, initialize these class on demand by calling
>>> InstanceKlass::initialize, which
>>> is a quick no-op if the class is already initialized.
>>>
>>> 2. The only initialization left is that of a global lock. So I
>>> renamed the function
>>> to SystemDictionaryShared::initialize_locks().
>>>
>>> 3. I moved the call of this function from
>>> SystemDictionary::compute_java_loaders() to
>>> SystemDictionary::initialize() where it seems to fit.
>>>
>>> Testing with hs-tiers 1 and 2.
>>>
>>> Thanks
>>> - Ioi
>>
>
More information about the hotspot-runtime-dev
mailing list