On Wed, 19 Jun 2024 16:50:22 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> We use 2 ParkEvent instances per thread. The ParkEvent objects are never >> freed, but they are recycled when a thread dies, so the number of live >> ParkEvent instances is proportional to the maximum number of threads that >> were live at any time. >> >> On Windows, the ParkEvent object wraps a kernel Event object. Kernel objects >> are a limited and costly resource. In this PR, I replace the use of kernel >> events with user-space synchronization. >> >> The new implementation uses WaitOnAddress and WakeByAddressSingle methods to >> implement synchronization. The methods are available since Windows 8. We >> only support Windows 10 and newer, so OS support should not be a problem. >> >> WaitOnAddress was observed to return spuriously, so I added the necessary >> code to recalculate the timeout and continue waiting. >> >> Tier1-5 tests passed. Performance tests were... inconclusive. For example, >> `ThreadOnSpinWaitProducerConsumer` reported 30% better results, while >> `LockUnlock.testContendedLock` results were 50% worse. >> >> Thoughts? > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > Update comment so... it appears that the ToolTabSnippetTest failure is related to a change in timing caused by the change in synchronization. Before my PR, the test takes anywhere between 10-40 seconds to complete. After my PR, on the GHA machines it takes 23 minutes to complete. The slowdown is observable on Win 2016 and Win 2019, Win 2022 and the client systems (10 and 11) still finish under 30 seconds. The problem is related to the structure of this test; the test implements a producer/consumer pattern using Object.wait/notifyAll. The producer thread pushes text to a StringBuilder one byte at a time. The consumer thread runs a regex match on the StringBuilder contents. With the original implementation, the consumer thread only woke once every few hundred bytes, so the cost of the regex matching was reasonable. With the new implementation, it wakes up much more frequently, and the regex execution cost causes the test run time explosion. ------------- PR Comment: https://git.openjdk.org/jdk/pull/19778#issuecomment-2189679242
