-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replacing all synchronized methods with ReentrantLock #92
Comments
Hi @PRIESt512 ! A few years ago I tried replacing I'm surprised that I ran some tests today, again. If you want to see them, check this branch: #93 The results I obtained were not encouraging. Unfortunately. This is the benchmark result using
And this using
I also tried a
For now, I think the synchronized keyword should stay. Regards, DEVICE: |
Thanks for the answer! I checked yours branch: VM version: JDK 17.0.8.1, OpenJDK 64-Bit Server VM, 17.0.8.1+0 (M1 Max, macOS 14.0)
Identical results on grallVM 21. Here graal no longer gives an improvement, I think because P.S. Perhaps due to the fact that the arm architecture has weaker memory semantics, this gives better performance on arm processors... Ideally, you need to look at the assembler code of the JIT machine on two platforms. Interesting case... I'll check this thread on the server later. On the server there is an AMD EPYC 7713 processor (16 cores). |
It is also possible for CAS (Compare-And-Swap) operations to be cheaper by m1 max... Here is some old information on Synchronized vs ReentrantLock: |
Small addition:
The last 2 measurements indicate that with ReentrantLock 'synchronized' there is no effect. Because synchronized on one thread is very productive (1 measurement). |
Results from the server - CentOS Linux 8, cpu - AMD EPYC 7713 (16 cores).
checked yours branch:
x64 arm64 Final I'll most likely leave the shell as it's more static in the results. |
I reimplemented the I'll try to find time next week to look for bugs, review unit tests, and update documentation. I'm not sure if the code is ready to be merged. New benchmarks here: https://github.com/f4b6a3/uuid-creator/actions/runs/6528172952 Thank you for your help! |
A little addition for Quote from Oracle: Pinning does not make an application incorrect, but it might hinder its scalability. Try avoiding frequent and long-lived pinning by revising synchronized blocks or methods that run frequently and guarding potentially long I/O operations with java.util.concurrent.locks.ReentrantLock. More details about virtual threads Some popular projects also refused to use synchronized` - Postgres JDBC Driver It wouldn't be a big surprise if |
The UUID version 7 generator was refactored. Now it uses reentrant lock. The version v5.3.6 will is now available. Thanks for your help and advice! |
Hi,
In my project I use UuidCreator.getTimeOrderedEpochPlus1(). It's a hot call. Looking at the features of the JVM, I slightly modified the call.
The final results are as follows (JMH):
standard method call in OpenJDK 17-21 -> 7-8_000 ops/ms (4 Threads)
standard method call in Oracle GraalVM 21 -> 12-13_000 ops/ms (4 Threads). Cunning jit-fox.
call with wrapper OpenJDK 17-21 -> 17-18_000 ops/ms (4 Threads)
call with wrapper Oracle GraalVM 21 -> 20_000 ops/ms (4 Threads)
Plus, you should take into account the nuances of the JDK after version 15, see - https://dev.to/vbochenin/java-17-migration-bias-locks-regression-2c5m
Using -XX:+UseBiasedLocking doesn't make much difference in JDK 17. In 21 this option is completely removed.
It is reasonable to assume that modifying all synchronized calls should have a good effect on the library's performance.
Device - MacBookPro M1 Max
The text was updated successfully, but these errors were encountered: