Skip to content

Commit

Permalink
JBR-6979 Modernize more WaitForSingleObject on Windows
Browse files Browse the repository at this point in the history
Use -XX:+UnlockExperimentalVMOptions -XX:-UseModernSynchAPI
to switch back to the original implementation
  • Loading branch information
mkartashev authored and YaaZ committed Mar 3, 2025
1 parent 051c493 commit a519c00
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 58 deletions.
2 changes: 1 addition & 1 deletion make/autoconf/libraries.m4
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ AC_DEFUN_ONCE([LIB_SETUP_LIBRARIES],
if test "x$OPENJDK_TARGET_OS" = xwindows; then
BASIC_JVM_LIBS="$BASIC_JVM_LIBS kernel32.lib user32.lib gdi32.lib winspool.lib \
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib powrprof.lib uuid.lib \
ws2_32.lib winmm.lib version.lib psapi.lib"
ws2_32.lib winmm.lib version.lib psapi.lib Synchronization.lib"
fi
LIB_SETUP_JVM_LIBS(BUILD)
LIB_SETUP_JVM_LIBS(TARGET)
Expand Down
5 changes: 4 additions & 1 deletion src/hotspot/os/windows/globals_windows.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ product(bool, UseOSErrorReporting, false, \
"Let VM fatal error propagate to the OS (ie. WER on Windows)") \
\
product(bool, UseCriticalSection, true, EXPERIMENTAL, \
"Use the critical section API instead of WaitForSingleObject")
"Use the critical section API instead of WaitForSingleObject") \
\
product(bool, UseModernSynchAPI, true, EXPERIMENTAL, \
"Use more modern WinAPI for synchronization instead of WaitForSingleObject")

// end of RUNTIME_OS_FLAGS

Expand Down
162 changes: 113 additions & 49 deletions src/hotspot/os/windows/os_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5617,7 +5617,9 @@ int PlatformEvent::park(jlong Millis) {
// 1 => 0 : pass - return immediately
// 0 => -1 : block; then set _Event to 0 before returning

guarantee(_ParkHandle != nullptr , "Invariant");
if (!UseModernSynchAPI) {
guarantee(_ParkHandle != nullptr , "Invariant");
}
guarantee(Millis > 0 , "Invariant");

// CONSIDER: defer assigning a CreateEvent() handle to the Event until
Expand Down Expand Up @@ -5649,25 +5651,37 @@ int PlatformEvent::park(jlong Millis) {
// adjust Millis accordingly if we encounter a spurious wakeup.

const int MAXTIMEOUT = 0x10000000;
DWORD rv = WAIT_TIMEOUT;
while (_Event < 0 && Millis > 0) {
DWORD prd = Millis; // set prd = MAX (Millis, MAXTIMEOUT)
if (Millis > MAXTIMEOUT) {
prd = MAXTIMEOUT;
}

if (UseModernSynchAPI) {
HighResolutionInterval *phri = nullptr;
if (!ForceTimeHighResolution) {
phri = new HighResolutionInterval(prd);
phri = new HighResolutionInterval(Millis);
}
rv = ::WaitForSingleObject(_ParkHandle, prd);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
if (rv == WAIT_TIMEOUT) {
Millis -= prd;
if ((v = Atomic::load_acquire(&_Event)) < 0) {
::WaitOnAddress(&_Event, &v, sizeof(_Event), Millis);
}
delete phri; // if it is null, harmless
} else {
DWORD rv = WAIT_TIMEOUT;
while (_Event < 0 && Millis > 0) {
DWORD prd = Millis; // set prd = MAX (Millis, MAXTIMEOUT)
if (Millis > MAXTIMEOUT) {
prd = MAXTIMEOUT;
}
HighResolutionInterval *phri = nullptr;
if (!ForceTimeHighResolution) {
phri = new HighResolutionInterval(prd);
}
rv = ::WaitForSingleObject(_ParkHandle, prd);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
if (rv == WAIT_TIMEOUT) {
Millis -= prd;
}
delete phri; // if it is null, harmless
}
v = _Event;
}
v = _Event;
_Event = 0;
// see comment at end of PlatformEvent::park() below:
OrderAccess::fence();
Expand All @@ -5683,7 +5697,9 @@ void PlatformEvent::park() {
// 1 => 0 : pass - return immediately
// 0 => -1 : block; then set _Event to 0 before returning

guarantee(_ParkHandle != nullptr, "Invariant");
if (!UseModernSynchAPI) {
guarantee(_ParkHandle != nullptr, "Invariant");
}
// Invariant: Only the thread associated with the Event/PlatformEvent
// may call park().
// Consider: use atomic decrement instead of CAS-loop
Expand All @@ -5695,21 +5711,35 @@ void PlatformEvent::park() {
guarantee((v == 0) || (v == 1), "invariant");
if (v != 0) return;

// Do this the hard way by blocking ...
// TODO: consider a brief spin here, gated on the success of recent
// spin attempts by this thread.
while (_Event < 0) {
// The following code is only here to maintain the
// characteristics/performance from when an ObjectMonitor
// "responsible" thread used to issue timed parks.
HighResolutionInterval *phri = nullptr;
if (!ForceTimeHighResolution) {
phri = new HighResolutionInterval((jlong)1);
if (UseModernSynchAPI) {
while ((v = Atomic::load_acquire(&_Event)) < 0) {
// The following code is only here to maintain the
// characteristics/performance from when an ObjectMonitor
// "responsible" thread used to issue timed parks.
HighResolutionInterval *phri = nullptr;
if (!ForceTimeHighResolution) {
phri = new HighResolutionInterval((jlong)1);
}
::WaitOnAddress(&_Event, &v, sizeof(_Event), INFINITE);
delete phri; // if it is null, harmless
}
} else {
// Do this the hard way by blocking ...
// TODO: consider a brief spin here, gated on the success of recent
// spin attempts by this thread.
while (_Event < 0) {
// The following code is only here to maintain the
// characteristics/performance from when an ObjectMonitor
// "responsible" thread used to issue timed parks.
HighResolutionInterval *phri = nullptr;
if (!ForceTimeHighResolution) {
phri = new HighResolutionInterval((jlong)1);
}
DWORD rv = ::WaitForSingleObject(_ParkHandle, INFINITE);
delete phri; // if it is null, harmless
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", rv);
}
DWORD rv = ::WaitForSingleObject(_ParkHandle, INFINITE);
delete phri; // if it is null, harmless
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", rv);
}

// Usually we'll find _Event == 0 at this point, but as
Expand All @@ -5721,7 +5751,9 @@ void PlatformEvent::park() {
}

void PlatformEvent::unpark() {
guarantee(_ParkHandle != nullptr, "Invariant");
if (!UseModernSynchAPI) {
guarantee(_ParkHandle != nullptr, "Invariant");
}

// Transitions for _Event:
// 0 => 1 : just return
Expand All @@ -5737,9 +5769,14 @@ void PlatformEvent::unpark() {
// from the first park() call after an unpark() call which will help
// shake out uses of park() and unpark() without condition variables.

if (Atomic::xchg(&_Event, 1) >= 0) return;

::SetEvent(_ParkHandle);
if (UseModernSynchAPI) {
if (Atomic::xchg(&_Event, 1) >= 0) return;
// Changed from -1 to 1; the target thread's WaitOnAddress() must return now
::WakeByAddressAll((PVOID) &_Event);
} else {
if (Atomic::xchg(&_Event, 1) >= 0) return;
::SetEvent(_ParkHandle);
}
}


Expand All @@ -5751,7 +5788,6 @@ void PlatformEvent::unpark() {
// use them directly.

void Parker::park(bool isAbsolute, jlong time) {
guarantee(_ParkHandle != nullptr, "invariant");
// First, demultiplex/decode time arguments
if (time < 0) { // don't wait
return;
Expand All @@ -5771,32 +5807,60 @@ void Parker::park(bool isAbsolute, jlong time) {

JavaThread* thread = JavaThread::current();

// Don't wait if interrupted or already triggered
if (thread->is_interrupted(false)) {
ResetEvent(_ParkHandle);
return;
if (UseModernSynchAPI) {
// Don't wait if interrupted or already triggered
if (thread->is_interrupted(false)) {
Atomic::release_store(&_TargetValue, 0);
return;
} else {
int curHandle = Atomic::load_acquire(&_TargetValue);
if (curHandle > 0) {
// Already unparked
Atomic::release_store(&_TargetValue, 0);
return;
} else {
ThreadBlockInVM tbivm(thread);
OSThreadWaitState osts(thread->osthread(), false /* not Object.wait() */);

// Spurios wakeups are fine as per Unsafe.park() promise
::WaitOnAddress(&_TargetValue, &curHandle, sizeof(_TargetValue), time);
Atomic::release_store(&_TargetValue, 0);
}
}
} else {
DWORD rv = WaitForSingleObject(_ParkHandle, 0);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
if (rv == WAIT_OBJECT_0) {
guarantee(_ParkHandle != nullptr, "invariant");
// Don't wait if interrupted or already triggered
if (thread->is_interrupted(false)) {
ResetEvent(_ParkHandle);
return;
} else {
ThreadBlockInVM tbivm(thread);
OSThreadWaitState osts(thread->osthread(), false /* not Object.wait() */);

rv = WaitForSingleObject(_ParkHandle, time);
DWORD rv = WaitForSingleObject(_ParkHandle, 0);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
ResetEvent(_ParkHandle);
if (rv == WAIT_OBJECT_0) {
ResetEvent(_ParkHandle);
return;
} else {
ThreadBlockInVM tbivm(thread);
OSThreadWaitState osts(thread->osthread(), false /* not Object.wait() */);

rv = WaitForSingleObject(_ParkHandle, time);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
ResetEvent(_ParkHandle);
}
}
}
}

void Parker::unpark() {
guarantee(_ParkHandle != nullptr, "invariant");
SetEvent(_ParkHandle);
if (UseModernSynchAPI) {
Atomic::release_store(&_TargetValue, 1);
::WakeByAddressAll(&_TargetValue);
} else {
guarantee(_ParkHandle != nullptr, "invariant");
SetEvent(_ParkHandle);
}
}

// Platform Mutex/Monitor implementation
Expand Down
23 changes: 16 additions & 7 deletions src/hotspot/os/windows/park_windows.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ class PlatformEvent : public CHeapObj<mtSynchronizer> {
public:
PlatformEvent() {
_Event = 0 ;
_ParkHandle = CreateEvent (nullptr, false, false, nullptr) ;
guarantee (_ParkHandle != nullptr, "invariant") ;
if (!UseModernSynchAPI) {
_ParkHandle = CreateEvent (nullptr, false, false, nullptr) ;
guarantee (_ParkHandle != nullptr, "invariant") ;
}
}

// Exercise caution using reset() and fired() - they may require MEMBARs
Expand All @@ -57,16 +59,23 @@ class PlatformEvent : public CHeapObj<mtSynchronizer> {
class PlatformParker {
NONCOPYABLE(PlatformParker);

protected:
protected:
HANDLE _ParkHandle;
int _TargetValue;

public:
public:
PlatformParker() {
_ParkHandle = CreateEvent (nullptr, true, false, nullptr) ;
guarantee(_ParkHandle != nullptr, "invariant") ;
if (UseModernSynchAPI) {
_TargetValue = 0;
} else {
_ParkHandle = CreateEvent (nullptr, true, false, nullptr) ;
guarantee(_ParkHandle != nullptr, "invariant") ;
}
}
~PlatformParker() {
CloseHandle(_ParkHandle);
if (! UseModernSynchAPI) {
CloseHandle(_ParkHandle);
}
}
};

Expand Down
68 changes: 68 additions & 0 deletions test/jdk/jb/hotspot/Synch.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright 2025 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


/*
* @test
* @summary Verifies that the test finishes in due time
* @library /test/lib
* @run main/othervm Synch
*/
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

public class Synch {
private static int counter = 0;

private static final Object lock = new Object();

private static final int NUM_THREADS = 8;

private static final long TEST_DURATION_SECONDS = 30;

public static void main(String[] args) throws InterruptedException {
ExecutorService executor = Executors.newFixedThreadPool(NUM_THREADS);

for (int i = 0; i < NUM_THREADS; i++) {
executor.submit(new AccessTask());
}

System.out.println("Warmup");
TimeUnit.SECONDS.sleep(TEST_DURATION_SECONDS);
synchronized (lock) {
counter = 0;
}

System.out.println("Starting the test...");
TimeUnit.SECONDS.sleep(TEST_DURATION_SECONDS);

executor.shutdownNow();
System.out.println("Total synchronized accesses per millisecond: " + (long)((double)counter / (TEST_DURATION_SECONDS * 1000)));
}

// Task performed by each thread
static class AccessTask implements Runnable {
@Override
public void run() {
while (!Thread.currentThread().isInterrupted()) {
synchronized (lock) {
counter++;
}
}
}
}
}

0 comments on commit a519c00

Please sign in to comment.