Hi David,
Thanks very much for investigating this and posting your recommendations.
On 01/04/2019 08:15, David Holmes wrote:
>> I did some analysis of the class loading and initialization sequence
>> and added my suggestions to bug report. In summary loading seems
>> somewhat immaterial so I suggest:
>>
>> - javaClasses.hpp: Add UnsafeConstants at the end of
>> BASIC_JAVA_CLASSES_DO_PART2
>> - systemDictionary.hpp: Add UnsafeConstants immediately before Unsafe
>>
>> Then for init:
>> - thread.cpp: initialize UnsafeConstants immediately after j.l.Module
I have done all of the above. Also, in javaClasses.hpp to keep things
uniform I moved the class decl for jdk_internal_misc_UnsafeConstants to
follow that of class java_..._AbstractOwnableSynchronizer i.e. the class
decl order matches the order of the do_klass statements.
>> It would be desirable to detect if we happen to execute the <clinit>
>> earlier (by accident) so I suggest adding a "not initialized"
>> assertion prior to your code calling initialize_class. Actually that
>> might be a useful addition to the initialize_class method, as if it
>> fires it means we're not initializing in the expected order ... I'll
>> run a little adding that ...
>
> I meant to say "run a little test". Turns out you can't put the
> assertion in initialize_class as the initialization can vary for some of
> the exception classes (at least) depending on VM flags used (e.g. -Xrs,
> and I think certain logging options).
Ok, thanks. I have added the following immediately before calling
initialize_class.
#ifdef ASSERT
InstanceKlass *k = SystemDictionary::UnsafeConstants_klass();
assert(k->is_not_initialized(), "UnsafeConstants should not already be
initialized");
#endif
. . .
I'll wait for feedback from Coleen before sending an updated webrev.
regards,
Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
