On 12/04/2021 8:53 pm, Robbin Ehn wrote:
On Wed, 7 Apr 2021 13:57:50 GMT, Robbin Ehn <r...@openjdk.org> wrote:
src/hotspot/share/runtime/thread.inline.hpp line 207:
205: }
206:
207: inline void JavaThread::set_terminated(TerminatedTypes t) {
I prefer set_terminated(arg) over the new set of methods.
We had two methods:
void set_terminated(TerminatedTypes t);
void set_terminated_value();
Terminated is part of the name of the method, the name of the flag, the name of
the type and part of the names of two of the states, which is very confusing.
I'll admit the API is not that clean but I would expect the method, the
flag, the type, to all share a common name as they are all related to
the same underlying thing: the termination state.
set_terminated_value() seems completely unnecessary as a special-case.
Also the setters now match the queries:
E.g.
`bool is_exiting()`
The queries do not indicate in any sense that they are queries on the
terminated flag.
The state flag is an implementation detail from query POV.
So to be consistent e.g. "set_exiting()" also hides the fact that we keep track
of this with a flag.
The point of the flag is that it is tracking a singular termination
state with a progression from _not_terminated, through _thread_exiting,
to _thread terminated, or the special state _vm_terminated (though I'm
not sure why we need that ...)
Please advise :) , I can roll back if you insist!
There is always room for improvement with these things, but the changes
you are making to this part of the thread API seem completely unrelated
to thread suspension, so I'd ask that you please roll them back, for now
at least.
Thanks,
David
-------------
PR: https://git.openjdk.java.net/jdk/pull/3191
