RFR: 8261479: CDS runtime code should check exceptions [v3]
David Holmes
dholmes at openjdk.java.net
Tue Mar 23 04:31:41 UTC 2021
On Tue, 23 Mar 2021 01:31:52 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:
>> Proposed changes include:
>>
>> - For the functions which could throw exceptions, the callers should pass in CHECK instead of THREAD.
>>
>> - For the functions which won't throw exceptions, put THREAD as the first parameter.
>> E.g.`static bool add_unregistered_class(Thread* current, InstanceKlass* k);`
>>
>> - For the functions which won't throw exceptions and don't use THREAD, just simply get rid of the TRAPS parameter.
>>
>> Testing: passed tiers 1 and 2, in-progress tiers 3 and 4.
>
> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>
> @iklam review comment
Hi Calvin,
This looks much better - thanks!
A couple of comments below, but only one of substance. I'll approve as-is but I think Coleen would want changes per my comment.
David
src/hotspot/share/classfile/classLoader.cpp line 290:
> 288: u1* ClassPathZipEntry::open_entry(Thread* current, const char* name, jint* filesize, bool nul_terminate) {
> 289: // enable call to C land
> 290: JavaThread* thread = current->as_Java_thread();
No need to introduce a local for a single use.
src/hotspot/share/classfile/klassFactory.cpp line 176:
> 174: assert(THREAD->is_Java_thread(), "must be a JavaThread");
> 175:
> 176: JavaThread* current = THREAD->as_Java_thread();
You can delete the assert at line 174 as the same assert is inside as_Java_thread().
But I think this is the kind of change that Coleen objects to: introducing a new local variable just to avoid using THREAD. The changes in this file can probably be reverted.
-------------
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3097
More information about the hotspot-runtime-dev
mailing list