JDK-8230459: Test failed to resume JVMCI CompilerThread

Kim Barrett kim.barrett at oracle.com
Mon Oct 28 19:40:08 UTC 2019


> On Oct 28, 2019, at 10:06 AM, Doerr, Martin <martin.doerr at sap.com> wrote:
> @Kim:
> Thanks for looking at the handle related parts. It's ok if you don't want to be a reviewer of the whole change.

I’ll reply to the pending parts, and drop out.  I think the handle related parts are
going in the right direction now.

>> I think it's weird that can_remove() is a predicate with optional side
>> effects.  I think it would be simpler to have it be a pure predicate,
>> and have the one caller with do_it = true perform the updates.  That
>> should include NULLing out the handle pointer (perhaps debug-only, but
>> it doesn't cost much to cleanly maintain the data structure).
> Nevertheless, it has the advantage that it enforces the update to be consistent.
> A caller could use it without holding the lock or mess it up otherwise.
> In addition, I don't what to change that as part of this fix.

Understood, though I think I recall the caller with do_it = true is holding the appropriate lock
around the call.

>> So far as I can tell, THREAD == NULL here.
> This is a very tricky part (not my invention):
> EXCEPTION_MARK contains an ExceptionMark constructor call which sets __the_thread__ to Thread::current().
> I don't want to publish my opinion about this ��

Oh, that’s interesting…  I know exceptions.hpp contains some exciting code, but I’ve so
far avoided looking at it closely.  That stuff doesn’t come up much in gc code :)

> @David:
> Seems like this option is preferred over option 3 (possibly_add_compiler_threads part of webrev.02 and leave the initialization as is).
> So when you're ok with it, I'll request a 2nd review from the compiler folks (I should change the subject to contain RFR).

Handle usage here looks okay to me now.  I think JVMCI global handles could be used instead,
but I’ll leave that up to others.



More information about the hotspot-compiler-dev mailing list