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

[Ioi Lam]

On Wed, 24 Mar 2021 16:20:30 GMT, Coleen Phillimore <coleenp 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)

Changes requested by iklam (Reviewer).

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.

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;
}

src/hotspot/share/classfile/systemDictionary.cpp line 1842:

> 1840:       klass = Universe::typeArrayKlassObj(t);
> 1841:     } else {
> 1842:       MutexLocker mu(SystemDictionary_lock);

Since this is a clean up RFE, I think it's better to avoid changes that may impact performance. I would avoid adding calls to Thread::current() -- except for cases inside logging code. Maybe change TRAPS to Thread* current and move it to the first parameter? I.e., how you changed SystemDictionaryShared::check_linking_constraints().

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(), ".....");
}

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

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