On Fri, 2 Jul 2021 07:03:50 GMT, David Holmes <dhol...@openjdk.org> wrote:
> Please review this simple refactoring to share the common code used to create > j.l.Thread instances for the internal VM JavaThreads. (Also fix a missing > space from my previous change in thread.cpp.) > > It is all very straight-forward. > > Compiler folk I can go one step further and remove > CompileBroker::create_thread_oop if desired. > > Testing: tiers 1-3, GHA > > Thanks, > David Hi, David, I found you wrote the same comment twice. one is in thread.hpp. the other one is thread.cpp. Is it necessary? if we only need to keep one copy, should we write comments in header file or impl file? The second thing your change is not exactly same for attachListener.cpp. Originally, it returns if JavaCalls::construct_new_instance() has trouble, ie. `has_init_error(THREAD)` returns true. In your change, you call "add group" but check error later. I guess call "call_special" should throw a java exception instead of just setting its state "AL_NOT_INITIALIZED". Does it matter? Other part of code looks good. ------------- PR: https://git.openjdk.java.net/jdk/pull/4663
