RFR: 8264126: Remove TRAPS/THREAD parameter for class loading functions

Coleen Phillimore coleenp at openjdk.java.net
Wed Mar 24 19:35:44 UTC 2021


On Wed, 24 Mar 2021 18:52:56 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> find_constrained_instance_or_array_klass only passes THREAD so that it can be used in a MutexLocker for SystemDictionary_lock. This can use the MutexLocker that gets Thread::current() without any harm to performance.
>> 
>> The other functions add_loader_constraint, record_linking_constraints, and check_signature_loaders fall out from that.
>> 
>> check_signature_loaders should throw an exception but it unfortunately makes the caller construct the exception message so it doesn't.
>> 
>> Also: is_shared_class_visible{_impl}
>> 
>> Tested with tier1 on 4 Oracle platforms (in progress)
>
> src/hotspot/share/ci/ciEnv.cpp line 428:
> 
>> 426:   }
>> 427: 
>> 428:   Handle loader;
> 
> I think it's better to get rid of the EXCEPTION_CONTEXT above, and add
> 
> Thread* current = Thread::current();
> 
> Also, the extensive use of EXCEPTION_CONTEXT in the JVMCI code should be reviewed. I think they probably need to be either removed or changed to EXCEPTION_MARK.

Ok, that works for ciEnv.cpp.  I'd rather not change the JVMCI code any more and leave it up to the maintainers of that code.  It's just a cleanliness issue not correctness.

> src/hotspot/share/classfile/systemDictionary.cpp line 1234:
> 
>> 1232:   Symbol* class_name = ik->name();
>> 1233: 
>> 1234:   bool visible = is_shared_class_visible(class_name, ik, pkg_entry, class_loader);
> 
> No longer need the local `visible`. Such locals were needed because we couldn't do 
> 
> if (foobar(a, b, c, CHECK_NULL)) {
>  return NULL;
> }

ok.

> src/hotspot/share/classfile/systemDictionaryShared.cpp line 1864:
> 
>> 1862:   }
>> 1863: 
>> 1864:   if (Thread::current()->is_VM_thread()) {
> 
> For performance, maybe it's better:
> if (DynamicDumpSharedSpaces) {
>   if (Thread::current()->is_VM_thread()) {
>     return;
>   }
> } else {
>   assert(!Thread::current()->is_VM_thread(), ".....");
> }

So we always dump dynamic shared spaces from the VMThread?

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

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


More information about the hotspot-dev mailing list