Hi Serguei,
Thanks for taking a look.
On 10/03/2021 9:53 am, Serguei Spitsyn wrote:
On Tue, 9 Mar 2021 18:52:27 GMT, Chris Plummer <[email protected]> wrote:
David Holmes has updated the pull request incrementally with three additional
commits since the last revision:
- Fix typo
- Fixed up BiasedLocking code in ObjectSynchronizer::enter
- iFixed alignment in macro
JVMTI changes look good.
Hi David,
Nice unification, looks great.
A couple of comments.
src/hotspot/share/runtime/synchronizer.cpp
638 int ObjectSynchronizer::wait(Handle obj, jlong millis, TRAPS) {
639 JavaThread* current = THREAD->as_Java_thread();
. . .
652 DTRACE_MONITOR_WAIT_PROBE(monitor, obj(), current, millis);
653 monitor->wait(millis, true, THREAD); // Not CHECK as we need following
code
Is it intentional to use `THREAD` instead of `current` at line 653?
Yes, purely as a documentation aid - the "THREAD" usage indicates the
wait() can lead to an exception. (Once TRAPS is defined using JavaThread
the "current" variable won't be needed.)
1141 bool ObjectSynchronizer::request_deflate_idle_monitors() {
1142 Thread* current = Thread::current();
. . .
1158 if (current->is_Java_thread()) {
1159 // JavaThread has to honor the blocking protocol.
1160 ThreadBlockInVM tbivm(current->as_Java_thread());
. . .
Would it better to define `current` as at the line 639?
There can be similar cases, e.g. the `deflate_idle_monitors()`.
I'm not sure what you mean - the current thread in these calls need not
be a JavaThread ... hmmm actually ... since JDK-8254029
request_deflate_idle_monitors is only used by WhiteBox for testing, so
the current thread must be a JavaThread (previously it could be VMThread
during VM shutdown). But not so for deflate_idle_monitors
// This function is called by the MonitorDeflationThread to deflate
// ObjectMonitors. It is also called via do_final_audit_and_print_stats()
// by the VMThread.
size_t ObjectSynchronizer::deflate_idle_monitors() {
I'll make an adjustment to request_deflate and re-test.
Thanks,
David
-Serguei
-------------
PR: https://git.openjdk.java.net/jdk/pull/2802
