RFR: 8225788: Dead code in thread and safepoint
robbin.ehn at oracle.com
Fri Aug 9 13:16:54 UTC 2019
Hi David, Dan, Coleen,
On 2019-06-21 07:52, David Holmes wrote:
> Hi Robbin,
> Sorry for the extra long delay on getting back to this.
Sorry for super extra long delay :)
I addressed below and re-based it:
(sorry no inc)
I think the only loose end is the removal of:
void trace_oops() PRODUCT_RETURN;
> You deleted JavaThread::java_priority(), but in os.cpp we have:
> // The mapping from OS priority back to Java priority may be inexact because
> // Java priorities can map M:1 with native priorities. If you want the definite
> // Java priority then use JavaThread::java_priority()
> So at a minimum the comment has to be updated. But pushing on ThreadPriority a
> little harder ... in thread.hpp we have:
> // Prepare thread and add to priority queue. If a priority is
> // not specified, use the priority of the thread object. Threads_lock
> // must be held while this function is called.
> void prepare(jobject jni_thread, ThreadPriority prio=NoPriority);
> The comment is out of date as no "priority queue" is involved anywhere. And the
> only call to JavaThread::prepare doesn't pass a ThreadPriority, so the parameter
> is unneeded and the logic in the implementation can just query the priority from
> the Thread object. That said, the code for that is:
> prio = java_lang_Thread::priority(thread_oop());
> and I would argue that that line should actually have been:
> prio = java_priority();
> thus making the function non-dead. As it is a single use I don't mind if the
> function goes but I think the assert it contained:
> assert(MinPriority <= priority && priority <= MaxPriority, "sanity check");
> should be added to the prepare() code.
> The use of ThreadPriority on set_calling_thread without it being used seems very
> odd. I'll track down when that code got introduced. The "priority" used for the
> VM op queues are not thread priorities so unclear how this came about.
> On 14/06/2019 6:08 am, David Holmes wrote:
>> Hi Robbin,
>> Sending out three RFRs after 6pm on my Friday night is not fair! I will look
>> at them all but not in detail till Monday :)
>> This one was much larger than expected. Some surprises hiding in there :)
>> There are a couple of things where my initial reaction is "hang on!" so I need
>> to look at a couple of things more closely. :)
>> E.g isn't JavaThread::trace_oops useful from the debugger?
>> It also seems to me that there is further dead code based on some of your
>> changes E.g. I don't see _orig_thread_state actually being read by anything.
>> On 14/06/2019 10:29 pm, Robbin Ehn wrote:
>>> Hi all, please review.
>>> Removing some dead code.
>>> Thanks, Robbin
More information about the hotspot-runtime-dev