RFR: 8264126: Remove TRAPS/THREAD parameter for class loading functions [v2]
David Holmes
dholmes at openjdk.java.net
Thu Mar 25 02:22:41 UTC 2021
On Wed, 24 Mar 2021 19:51:00 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)
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>
> Improvements suggested by Ioi.
Hi Coleen,
Generally this looks good, but I'm with Ioi here that where we pass through the current thread to avoid needing to manifest Thread::current down the stack (for MutexLockers or ResourceMarks etc) then I would prefer to see that kept - and if necessary relocate the Thread parameter to the beginning. In the past we have consciously added these thread parameters to avoid the Thread::current calls and I prefer not to see that undone. But it is very subjective - I removed one only needed for logging code for example.
Thanks,
David
src/hotspot/share/ci/ciEnv.cpp line 445:
> 443: {
> 444: ttyUnlocker ttyul; // release tty lock to avoid ordering problems
> 445: MutexLocker ml(Compile_lock);
We could pass current here too.
src/hotspot/share/jvmci/jvmciRuntime.cpp line 1268:
> 1266: {
> 1267: ttyUnlocker ttyul; // release tty lock to avoid ordering problems
> 1268: MutexLocker ml(Compile_lock);
Can pass THREAD here
-------------
Changes requested by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3176
More information about the hotspot-dev
mailing list