Biased locking Obsoletion
Patricio Chilano
patricio.chilano.mateo at oracle.com
Sat Nov 7 07:52:30 UTC 2020
Hi Martin,
Thanks for your feedback.
On 11/6/20 12:29 PM, Doerr, Martin wrote:
> Hi Patricio,
>
> seems like nobody wanted to be the first person to reply. So I just share a few thoughts.
>
> Unfortunately, I haven't heard any feedback from end users.
> If the Biased Locking Code removal is not urgent because it's in the way for something else, I'd slightly prefer to remove it early in JDK17.
>
> My impression is that modern workloads are fine without BL, so typical JDK15 users will probably not notice it was switched off.
>
> Some old workloads are heavily affected, like SPEC jvm98. See performance drop on Power9:
> http://cr.openjdk.java.net/~mdoerr/BiasedLocking_Power9.png
> and on Intel Xeon E5:
> http://cr.openjdk.java.net/~mdoerr/BiasedLocking_XeonE5.png
>
> Are there any plans for mitigations?
> If so, it would be nice to implement them before finally removing BL.
SPECjvm98 uses some classes that synchronize on pretty much every access
and almost all workloads are single-threaded. To give you a couple of
examples, I logged all synchronizations attempts from _209_db, _228_jack
and _202_jess, benchmarks for which I saw regressions from
10%(_202_jess) up to 25%-30%(_209_db), similar to the results you sent.
Here are the results showing the amount of times a thread synchronized
on objects of a given class (run a single iteration with -g -M1 -s100):
_209_db:
Count Thread Class
56255655 0x00007f9a18026dd0 java.util.Vector
24252 0x00007f9a18026dd0 spec.io.FileInputStream
15326 0x00007f9a18026dd0 java.lang.StringBuffer
5203 0x00007f9a18026dd0 java.io.ByteArrayOutputStream
5203 0x00007f9a18026dd0 java.io.ByteArrayInputStream
4088 0x00007f9a18026dd0 java.io.OutputStreamWriter
1643 0x00007f9a18026dd0 spec.io.PrintStream
931 0x00007f9a18026dd0 java.io.BufferedOutputStream
633 0x00007f9a18026dd0 spec.io.TableOfExistingFiles
625 0x00007f9a18026dd0 java.io.PrintStream
369 0x00007f9a18026dd0
java.util.concurrent.ConcurrentHashMap$Node
(Full trace:
http://cr.openjdk.java.net/~pchilanomate/specjvm98synctests/_209_db.txt)
_228_jack
Count Thread Class
8349013 0x00007fba14026dd0 java.util.Vector
2808067 0x00007fba14026dd0 java.util.Hashtable
1173017 0x00007fba14026dd0 java.io.OutputStreamWriter
886454 0x00007fba14026dd0 java.lang.StringBuffer
580516 0x00007fba14026dd0 spec.benchmarks._228_jack.JackPrintStream
291017 0x00007fba14026dd0 spec.io.FileInputStream
1116 0x00007fba14026dd0 java.io.ByteArrayOutputStream
1116 0x00007fba14026dd0 java.io.ByteArrayInputStream
633 0x00007fba14026dd0 spec.io.TableOfExistingFiles
525 0x00007fba14026dd0
java.util.concurrent.ConcurrentHashMap$Node
414 0x00007fba14026dd0 java.io.FileDescriptor
(Full trace:
http://cr.openjdk.java.net/~pchilanomate/specjvm98synctests/_228_jack.txt)
_202_jess
Count Thread Class
3515114 0x00007f6ad4026dd0 java.util.Hashtable
1323387 0x00007f6ad4026dd0 java.util.Vector
43451 0x00007f6ad4026dd0 java.util.Stack
18920 0x00007f6ad4026dd0 java.lang.StringBuffer
14952 0x00007f6ad4026dd0 spec.io.FileInputStream
3811 0x00007f6ad4026dd0 java.io.OutputStreamWriter
2413 0x00007f6ad4026dd0 java.io.ByteArrayOutputStream
2413 0x00007f6ad4026dd0 java.io.ByteArrayInputStream
1623 0x00007f6ad4026dd0 spec.io.PrintStream
1380 0x00007f6ad4026dd0 java.lang.Class
847 0x00007f6ad4026dd0 java.io.FileDescriptor
814 0x00007f6ad4026dd0
java.util.concurrent.ConcurrentHashMap$Node
633 0x00007f6ad4026dd0 spec.io.TableOfExistingFiles
(Full trace:
http://cr.openjdk.java.net/~pchilanomate/specjvm98synctests/_202_jess.txt)
So all the work is done by the same JavaThread, and given the amount of
uncontended synchronization biased locking shines.
Not sure if the pictures you sent happen to be from those particular
benchmarks or others(_201_compress, _200_check, etc), but we could use
the same technique and find out which classes we are synchronizing on
that are causing the tests to prefer BL and whether the workload is
single or multithreaded. We can do the same for other benchmarks. Note:
you can also get those numbers with DiagnoseSyncOnPrimitiveWrappers, the
flag I added recently for Valhalla (plus some minor changes).
The same tests on benchmark _222_mpegaudio, for which I didn't see major
performance difference with biased locking disabled, showed the
following results (run a single iteration with -g -M1 -s100):
_222_mpegaudio
Count Thread Class
2165 0x00007fc4d0026dd0 spec.io.FileInputStream
790 0x00007fc4d0026dd0 java.lang.StringBuffer
633 0x00007fc4d0026dd0 spec.io.TableOfExistingFiles
581 0x00007fc4d0026dd0 java.util.concurrent.ConcurrentHashMap$Node
365 0x00007fc4d0026dd0 java.io.FileDescriptor
266 0x00007fc4d0026dd0 java.lang.Object
208 0x00007fc4d0026dd0 java.io.ByteArrayOutputStream
208 0x00007fc4d0026dd0 java.io.ByteArrayInputStream
(Full trace:
http://cr.openjdk.java.net/~pchilanomate/specjvm98synctests/_222_mpegaudio.txt)
Still single-threaded with many synchronized calls but not as much as
the above benchmarks, so it makes sense performance doesn't change much
with or without BL.
If the benchmarks above would be part of a real world app for which we
would be trying to solve a performance issue, I think the solution would
be to just drop java.util.Vector and java.util.Hashtable and use
java.util.ArrayList and java.util.HashMap instead. Also if using custom
classes, they shouldn't use synchronized keyword unless it's necessary.
Then, not only performance would not be affected but it's also likely it
will improve since we are not wasting time in unneeded monitorenter/exit
bytecodes. If the issue is that the JDK library only provides
synchronized classes, then I think we should have an unsynchronized
flavor too. Then there could be workflows that still benefit from BL
(although I tend to think the app code can probably be re-written so as
to avoid those unnecessary synchronization calls) but the question is
whether we want to support those cases.
I agree that since there is nothing urgent that needs BL to go away we
can push it to the next release instead. Then we will be having even
more time for feedback and/or fix any issues.
Thanks,
Patricio
> My 0.02$.
>
> Best regards,
> Martin
>
>
>> -----Original Message-----
>> From: hotspot-runtime-dev <hotspot-runtime-dev-retn at openjdk.java.net>
>> On Behalf Of David Holmes
>> Sent: Dienstag, 3. November 2020 22:30
>> To: Patricio Chilano <patricio.chilano.mateo at oracle.com>; hotspot-runtime-
>> dev at openjdk.java.net; hotspot-dev developers <hotspot-
>> dev at openjdk.java.net>
>> Subject: Re: Biased locking Obsoletion
>>
>> Expanding to hotspot-dev.
>>
>>
>> On 4/11/2020 7:08 am, Patricio Chilano wrote:
>>> Hi all,
>>>
>>> As discussed in 8231264, the idea was to switch biased locking to false
>>> by default and deprecate all related flags with the intent to remove the
>>> code in a future release unless compelling evidence showed that the code
>>> is worth maintaining.
>>> I see there is only one issue that was filed since biased locking was
>>> disabled by default (https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/542__;!!GqivPVa7Brio!NtUU4TDlOTLoheSxnnQbTo4M64nbmca-8qDkdq0V4MjIqCudEEvdyj8am8BnV257S3q83fUh7g$ ) that seems
>>> to have been addressed. As per 8231264 change, the code was set to be
>>> obsoleted in 16, so we are already in a position to remove biased
>>> locking code unless there are arguments for the contrary. The
>>> alternative would be to give more time and move biased locking
>>> obsoletion to a future release.
>>> Let me know your thoughts.
>>>
>>> Thanks,
>>>
>>> Patricio
More information about the hotspot-dev
mailing list