RFR(L) 8153224 Monitor deflation prolong safepoints (CR7/v2.07/10-for-jdk14)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Thu Oct 17 21:50:19 UTC 2019
Greetings,
The Async Monitor Deflation project is reaching the end game. I have no
changes planned for the project at this time so all that is left is code
review and any changes that results from those reviews.
Carsten and Roman! Time for you guys to chime in again on the code reviews.
I have attached the list of fixes from CR6 to CR7 instead of putting it
in the main body of this email.
Main bug URL:
JDK-8153224 Monitor deflation prolong safepoints
https://bugs.openjdk.java.net/browse/JDK-8153224
The project is currently baselined on jdk-14+19.
Here's the full webrev URL for those folks that want to see all of the
current Async Monitor Deflation code in one go (v2.07 full):
http://cr.openjdk.java.net/~dcubed/8153224-webrev/10-for-jdk14.v2.07.full
Some folks might want to see just what has changed since the last review
cycle so here's a webrev for that (v2.07 inc):
http://cr.openjdk.java.net/~dcubed/8153224-webrev/10-for-jdk14.v2.07.inc/
The OpenJDK wiki has been updated to match the CR7/v2.07/10-for-jdk14
changes:
https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
The jdk-14+18 based v2.07 version of the patch has been thru Mach5 tier[1-8]
testing on Oracle's usual set of platforms. It has also been through my
usual
set of stress testing on Linux-X64, macOSX and Solaris-X64 with the addition
of Robbin's "MoCrazy 1024" test running in parallel with the other tests in
my lab.
The jdk-14+19 based v2.07 version of the patch has been thru Mach5 tier[1-3]
test on Oracle's usual set of platforms. Mach5 tier[4-8] are in process.
I did another round of SPECjbb2015 testing in Oracle's Aurora
Performance lab
using using their tuned SPECjbb2015 Linux-X64 G1 configs:
- "base" is jdk-14+18
- "v2.07" is the latest version and includes C2 inc_om_ref_count()
support
on LP64 X64 and the new HandshakeAfterDeflateIdleMonitors option
- "off" is with -XX:-AsyncDeflateIdleMonitors specified
- "handshake" is with -XX:+HandshakeAfterDeflateIdleMonitors specified
hbIR hbIR
(max attempted) (settled) max-jOPS critical-jOPS runtime
--------------- --------- -------- ------------- -------
34282.00 30635.90 28831.30 20969.20 3841.30 base
34282.00 30973.00 29345.80 21025.20 3964.10 v2.07
34282.00 31105.60 29174.30 21074.00 3931.30
v2.07_handshake
34282.00 30789.70 27151.60 19839.10 3850.20 v2.07_off
- The Aurora Perf comparison tool reports:
Comparison max-jOPS critical-jOPS
---------------------- -------------------- --------------------
base vs 2.07 +1.78% (s, p=0.000) +0.27% (ns, p=0.790)
base vs 2.07_handshake +1.19% (s, p=0.007) +0.58% (ns, p=0.536)
base vs 2.07_off -5.83% (ns, p=0.394) -5.39% (ns, p=0.347)
(s) - significant (ns) - not-significant
- For historical comparison, the Aurora Perf comparision tool
reported for v2.06 with a baseline of jdk-13+31:
Comparison max-jOPS critical-jOPS
---------------------- -------------------- --------------------
base vs 2.06 -0.32% (ns, p=0.345) +0.71% (ns, p=0.646)
base vs 2.06_off +0.49% (ns, p=0.292) -1.21% (ns, p=0.481)
(s) - significant (ns) - not-significant
Thanks, in advance, for any questions, comments or suggestions.
Dan
On 8/28/19 5:02 PM, Daniel D. Daugherty wrote:
> Greetings,
>
> The Async Monitor Deflation project has rebased to JDK14 so it's time
> for our first code review in that new context!!
>
> I've been focused on changing the monitor list management code to be
> lock-free in order to make SPECjbb2015 happier. Of course with a change
> like that, it takes a while to chase down all the new and wonderful
> races. At this point, I have the code back to the same stability that
> I had with CR5/v2.05/8-for-jdk13.
>
> To lay the ground work for this round of review, I pushed the following
> two fixes to jdk/jdk earlier today:
>
> JDK-8230184 rename, whitespace, indent and comments changes in
> preparation
> for lock free Monitor lists
> https://bugs.openjdk.java.net/browse/JDK-8230184
>
> JDK-8230317 serviceability/sa/ClhsdbPrintStatics.java fails after
> 8230184
> https://bugs.openjdk.java.net/browse/JDK-8230317
>
> I have attached the list of fixes from CR5 to CR6 instead of putting
> in the main body of this email.
>
> Main bug URL:
>
> JDK-8153224 Monitor deflation prolong safepoints
> https://bugs.openjdk.java.net/browse/JDK-8153224
>
> The project is currently baselined on jdk-14+11 plus the fixes for
> JDK-8230184 and JDK-8230317.
>
> Here's the full webrev URL for those folks that want to see all of the
> current Async Monitor Deflation code in one go (v2.06 full):
>
> http://cr.openjdk.java.net/~dcubed/8153224-webrev/9-for-jdk14.v2.06.full/
>
>
> The primary focus of this review cycle is on the lock-free Monitor List
> management changes so here's a webrev for just that patch (v2.06c):
>
> http://cr.openjdk.java.net/~dcubed/8153224-webrev/9-for-jdk14.v2.06c.inc/
>
> The secondary focus of this review cycle is on the bug fixes that have
> been made since CR5/v2.05/8-for-jdk13 so here's a webrev for just that
> patch (v2.06b):
>
> http://cr.openjdk.java.net/~dcubed/8153224-webrev/9-for-jdk14.v2.06b.inc/
>
> The third and final bucket for this review cycle is the rename,
> whitespace,
> indent and comments changes made in preparation for lock free Monitor
> list
> management. Almost all of that was extracted into JDK-8230184 for the
> baseline so this bucket now has just a few comment changes relative to
> CR5/v2.05/8-for-jdk13. Here's a webrev for the remainder (v2.06a):
>
> http://cr.openjdk.java.net/~dcubed/8153224-webrev/9-for-jdk14.v2.06a.inc/
>
>
> Some folks might want to see just what has changed since the last review
> cycle so here's a webrev for that (v2.06 inc):
>
> http://cr.openjdk.java.net/~dcubed/8153224-webrev/9-for-jdk14.v2.06.inc/
>
>
> Last, but not least, some folks might want to see the code before the
> addition of lock-free Monitor List management so here's a webrev for
> that (v2.00 -> v2.05):
>
> http://cr.openjdk.java.net/~dcubed/8153224-webrev/9-for-jdk14.v2.05.inc/
>
> The OpenJDK wiki will need minor updates to match the CR6 changes:
>
> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
>
> but that should only be changes to describe per-thread list async monitor
> deflation being done by the ServiceThread.
>
> (I did update the OpenJDK wiki for the CR5 changes back on 2019.08.14)
>
> This version of the patch has been thru Mach5 tier[1-8] testing on
> Oracle's usual set of platforms. It has also been through my usual set
> of stress testing on Linux-X64, macOSX and Solaris-X64.
>
> I did a bunch of SPECjbb2015 testing in Oracle's Aurora Performance lab
> using using their tuned SPECjbb2015 Linux-X64 G1 configs. This was using
> this patch baselined on jdk-13+31 (for stability):
>
> hbIR hbIR
> (max attempted) (settled) max-jOPS critical-jOPS runtime
> --------------- --------- -------- ------------- -------
> 34282.00 28837.20 27905.20 19817.40 3658.10 base
> 34965.70 29798.80 27814.90 19959.00 3514.60 v2.06d
> 34282.00 29100.70 28042.50 19577.00 3701.90
> v2.06d_off
> 34282.00 29218.50 27562.80 19397.30 3657.60
> v2.06d_ocache
> 34965.70 29838.30 26512.40 19170.60 3569.90 v2.05
> 34282.00 28926.10 27734.00 19835.10 3588.40
> v2.05_off
>
> The "off" configs are with -XX:-AsyncDeflateIdleMonitors specified and
> the "ocache" config is with 128 byte cache line sizes instead of 64 byte
> cache lines sizes. "v2.06d" is the last set of changes that I made before
> those changes were distributed into the "v2.06a", "v2.06b" and "v2.06c"
> buckets for this review recycle.
>
>
> Thanks, in advance, for any questions, comments or suggestions.
>
> Dan
>
>
> On 7/11/19 3:49 PM, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I've been focused on chasing down and fixing the rare test failures
>> that only pop up rarely. So this round is primarily fixes for races
>> with a few additional fixes that came from Karen's review of CR4.
>> Thanks Karen!
>>
>> I have attached the list of fixes from CR4 to CR5 instead of putting
>> in the main body of this email.
>>
>> Main bug URL:
>>
>> JDK-8153224 Monitor deflation prolong safepoints
>> https://bugs.openjdk.java.net/browse/JDK-8153224
>>
>> The project is currently baselined on jdk-13+29. This will likely be
>> the last JDK13 baseline for this project and I'll roll to the JDK14
>> (jdk/jdk) repo soon...
>>
>> Here's the full webrev URL:
>>
>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/8-for-jdk13.full/
>>
>> Here's the incremental webrev URL:
>>
>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/8-for-jdk13.inc/
>>
>> I have not yet checked the OpenJDK wiki to see if it needs any updates
>> to match the CR5 changes:
>>
>> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
>>
>> (I did update the OpenJDK wiki for the CR4 changes back on 2019.06.26)
>>
>> This version of the patch has been thru Mach5 tier[1-3] testing on
>> Oracle's usual set of platforms. Mach5 tier[4-6] is running now and
>> Mach5 tier[78] will follow. I'll kick off the usual stress testing
>> on Linux-X64, macOSX and Solaris-X64 as those machines become available.
>> Since I haven't made any performance changes in this round, I'll only
>> be running SPECjbb2015 to gather the latest monitorinflation logs.
>>
>> Next up:
>>
>> - We're still seeing 4-5% lower performance with SPECjbb2015 on
>> Linux-X64 and we've determined that some of that comes from
>> contention on the gListLock. So I'm going to investigate removing
>> the gListLock. Yes, another lock free set of changes is coming!
>> - Of course, going lock free often causes new races and new failures
>> so that's a good reason for make those changes isolated in their
>> own round (and not holding up CR5/v2.05/8-for-jdk13 anymore).
>> - I finally have a potential fix for the Win* failure with
>> gc/g1/humongousObjects/TestHumongousClassLoader.java
>> but I haven't run it through Mach5 yet so it'll be in the next round.
>> - Some RTM tests were recently re-enabled in Mach5 and I'm seeing some
>> monitor related failures there. I suspect that I need to go take a
>> look at the C2 RTM macro assembler code and look for things that might
>> conflict if Async Monitor Deflation. If you're interested in that kind
>> of issue, then see the macroAssembler_x86.cpp sanity check that I
>> added in this round!
>>
>> Thanks, in advance, for any questions, comments or suggestions.
>>
>> Dan
>>
>>
>> On 5/26/19 8:30 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I have a fix for an issue that came up during performance testing.
>>> Many thanks to Robbin for diagnosing the issue in his SPECjbb2015
>>> experiments.
>>>
>>> Here's the list of changes from CR3 to CR4. The list is a bit
>>> verbose due to the complexity of the issue, but the changes
>>> themselves are not that big.
>>>
>>> Functional:
>>> - Change SafepointSynchronize::is_cleanup_needed() from calling
>>> ObjectSynchronizer::is_cleanup_needed() to calling
>>> ObjectSynchronizer::is_safepoint_deflation_needed():
>>> - is_safepoint_deflation_needed() returns the result of
>>> monitors_used_above_threshold() for safepoint based
>>> monitor deflation (!AsyncDeflateIdleMonitors).
>>> - For AsyncDeflateIdleMonitors, it only returns true if
>>> there is a special deflation request, e.g., System.gc()
>>> - This solves a bug where there are a bunch of Cleanup
>>> safepoints that simply request async deflation which
>>> keeps the async JavaThreads from making progress on
>>> their async deflation work.
>>> - Add AsyncDeflationInterval diagnostic option. Description:
>>> Async deflate idle monitors every so many milliseconds when
>>> MonitorUsedDeflationThreshold is exceeded (0 is off).
>>> - Replace ObjectSynchronizer::gOmShouldDeflateIdleMonitors() with
>>> ObjectSynchronizer::is_async_deflation_needed():
>>> - is_async_deflation_needed() returns true when
>>> is_async_cleanup_requested() is true or when
>>> monitors_used_above_threshold() is true (but no more often than
>>> AsyncDeflationInterval).
>>> - if AsyncDeflateIdleMonitors Service_lock->wait() now waits for
>>> at most GuaranteedSafepointInterval millis:
>>> - This allows is_async_deflation_needed() to be checked at
>>> the same interval as GuaranteedSafepointInterval.
>>> (default is 1000 millis/1 second)
>>> - Once is_async_deflation_needed() has returned true, it
>>> generally cannot return true for AsyncDeflationInterval.
>>> This is to prevent async deflation from swamping the
>>> ServiceThread.
>>> - The ServiceThread still handles async deflation of the global
>>> in-use list and now it also marks JavaThreads for async deflation
>>> of their in-use lists.
>>> - The ServiceThread will check for async deflation work every
>>> GuaranteedSafepointInterval.
>>> - A safepoint can still cause the ServiceThread to check for
>>> async deflation work via is_async_deflation_requested.
>>> - Refactor code from ObjectSynchronizer::is_cleanup_needed() into
>>> monitors_used_above_threshold() and remove is_cleanup_needed().
>>> - In addition to System.gc(), the VM_Exit VM op and the final
>>> VMThread safepoint now set the is_special_deflation_requested
>>> flag to reduce the in-use monitor population that is reported by
>>> ObjectSynchronizer::log_in_use_monitor_details() at VM exit.
>>>
>>> Test update:
>>> - test/hotspot/gtest/oops/test_markOop.cpp is updated to work with
>>> AsyncDeflateIdleMonitors.
>>>
>>> Collateral:
>>> - Add/clarify/update some logging messages.
>>>
>>> Cleanup:
>>> - Updated comments based on Karen's code review.
>>> - Change 'special cleanup' -> 'special deflation' and
>>> 'async cleanup' -> 'async deflation'.
>>> - comment and function name changes
>>> - Clarify MonitorUsedDeflationThreshold description;
>>>
>>>
>>> Main bug URL:
>>>
>>> JDK-8153224 Monitor deflation prolong safepoints
>>> https://bugs.openjdk.java.net/browse/JDK-8153224
>>>
>>> The project is currently baselined on jdk-13+22.
>>>
>>> Here's the full webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/7-for-jdk13.full/
>>>
>>> Here's the incremental webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/7-for-jdk13.inc/
>>>
>>> I have not updated the OpenJDK wiki to reflect the CR4 changes:
>>>
>>> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
>>>
>>> The wiki doesn't say a whole lot about the async deflation invocation
>>> mechanism so I have to figure out how to add that content.
>>>
>>> This version of the patch has been thru Mach5 tier[1-8] testing on
>>> Oracle's usual set of platforms. My Solaris-X64 stress kit run is
>>> running now. Kitchensink8H on product, fastdebug, and slowdebug bits
>>> are running on Linux-X64, MacOSX and Solaris-X64. I still have to run
>>> my stress kit on Linux-X64. I still have to run the SPECjbb2015
>>> baseline and CR4 runs on Linux-X64, MacOSX and Solaris-X64.
>>>
>>> Thanks, in advance, for any questions, comments or suggestions.
>>>
>>> Dan
>>>
>>> On 5/6/19 11:52 AM, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> I had some discussions with Karen about a race that was in the
>>>> ObjectMonitor::enter() code in CR2/v2.02/5-for-jdk13. This race was
>>>> theoretical and I had no test failures due to it. The fix is pretty
>>>> simple: remove the special case code for async deflation in the
>>>> ObjectMonitor::enter() function and rely solely on the ref_count
>>>> for ObjectMonitor::enter() protection.
>>>>
>>>> During those discussions Karen also floated the idea of using the
>>>> ref_count field instead of the contentions field for the Async
>>>> Monitor Deflation protocol. I decided to go ahead and code up that
>>>> change and I have run it through the usual stress and Mach5 testing
>>>> with no issues. It's also known as v2.03 (for those for with the
>>>> patches) and as webrev/6-for-jdk13 (for those with webrev URLs).
>>>> Sorry for all the names...
>>>>
>>>> Main bug URL:
>>>>
>>>> JDK-8153224 Monitor deflation prolong safepoints
>>>> https://bugs.openjdk.java.net/browse/JDK-8153224
>>>>
>>>> The project is currently baselined on jdk-13+18.
>>>>
>>>> Here's the full webrev URL:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/6-for-jdk13.full/
>>>>
>>>> Here's the incremental webrev URL:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/6-for-jdk13.inc/
>>>>
>>>> I have also updated the OpenJDK wiki to reflect the CR3 changes:
>>>>
>>>> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
>>>>
>>>> This version of the patch has been thru Mach5 tier[1-8] testing on
>>>> Oracle's usual set of platforms. My Solaris-X64 stress kit run had
>>>> no issues. Kitchensink8H on product, fastdebug, and slowdebug bits
>>>> had no failures on Linux-X64; MacOSX fastdebug and slowdebug and
>>>> Solaris-X64 release had the usual "Too large time diff" complaints.
>>>> 12 hour Inflate2 runs on product, fastdebug and slowdebug bits on
>>>> Linux-X64, MacOSX and Solaris-X64 had no failures. My Linux-X64
>>>> stress kit is running right now.
>>>>
>>>> I've done the SPECjbb2015 baseline and CR3 runs. I need to gather
>>>> the results and analyze them.
>>>>
>>>> Thanks, in advance, for any questions, comments or suggestions.
>>>>
>>>> Dan
>>>>
>>>>
>>>> On 4/25/19 12:38 PM, Daniel D. Daugherty wrote:
>>>>> Greetings,
>>>>>
>>>>> I have a small but important bug fix for the Async Monitor Deflation
>>>>> project ready to go. It's also known as v2.02 (for those for with the
>>>>> patches) and as webrev/5-for-jdk13 (for those with webrev URLs).
>>>>> Sorry
>>>>> for all the names...
>>>>>
>>>>> JDK-8222295 was pushed to jdk/jdk two days ago so that baseline patch
>>>>> is out of our hair.
>>>>>
>>>>> Main bug URL:
>>>>>
>>>>> JDK-8153224 Monitor deflation prolong safepoints
>>>>> https://bugs.openjdk.java.net/browse/JDK-8153224
>>>>>
>>>>> The project is currently baselined on jdk-13+17.
>>>>>
>>>>> Here's the full webrev URL:
>>>>>
>>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/5-for-jdk13.full/
>>>>>
>>>>> Here's the incremental webrev URL (JDK-8153224):
>>>>>
>>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/5-for-jdk13.inc/
>>>>>
>>>>> I still have to update the OpenJDK wiki to reflect the CR2 changes:
>>>>>
>>>>> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
>>>>>
>>>>> This version of the patch has been thru Mach5 tier[1-6] testing on
>>>>> Oracle's usual set of platforms. Mach5 tier[7-8] is running now.
>>>>> My stress kit is running on Solaris-X64 now. Kitchensink8H is running
>>>>> now on product, fastdebug, and slowdebug bits on Linux-X64, MacOSX
>>>>> and Solaris-X64. 12 hour Inflate2 runs are running now on product,
>>>>> fastdebug and slowdebug bits on Linux-X64, MacOSX and Solaris-X64.
>>>>> I'll start my my stress kit on Linux-X64 sometime on Sunday (after
>>>>> my jdk-13+18 stress run is done).
>>>>>
>>>>> I'll do SPECjbb2015 baseline and CR2 runs after all the stress
>>>>> testing is done.
>>>>>
>>>>> Thanks, in advance, for any questions, comments or suggestions.
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>> On 4/19/19 11:58 AM, Daniel D. Daugherty wrote:
>>>>>> Greetings,
>>>>>>
>>>>>> I finally have CR1 for the Async Monitor Deflation project ready to
>>>>>> go. It's also known as v2.01 (for those for with the patches) and as
>>>>>> webrev/4-for-jdk13 (for those with webrev URLs). Sorry for all the
>>>>>> names...
>>>>>>
>>>>>> Main bug URL:
>>>>>>
>>>>>> JDK-8153224 Monitor deflation prolong safepoints
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8153224
>>>>>>
>>>>>> Baseline bug fixes URL:
>>>>>>
>>>>>> JDK-8222295 more baseline cleanups from Async Monitor
>>>>>> Deflation project
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8222295
>>>>>>
>>>>>> The project is currently baselined on jdk-13+15.
>>>>>>
>>>>>> Here's the webrev for the latest baseline changes (JDK-8222295):
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/4-for-jdk13.8222295
>>>>>>
>>>>>>
>>>>>> Here's the full webrev URL (JDK-8153224 only):
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/4-for-jdk13.full/
>>>>>>
>>>>>> Here's the incremental webrev URL (JDK-8153224):
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/4-for-jdk13.inc/
>>>>>>
>>>>>> So I'm looking for reviews for both JDK-8222295 and the latest
>>>>>> version
>>>>>> of JDK-8153224...
>>>>>>
>>>>>> I still have to update the OpenJDK wiki to reflect the CR changes:
>>>>>>
>>>>>> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
>>>>>>
>>>>>>
>>>>>> This version of the patch has been thru Mach5 tier[1-3] testing on
>>>>>> Oracle's usual set of platforms. Mach5 tier[4-6] is running now and
>>>>>> Mach5 tier[78] will be run later today. My stress kit on Solaris-X64
>>>>>> is running now. Linux-X64 stress testing will start on Sunday. I'm
>>>>>> planning to do Kitchensink runs, SPECjbb2015 runs and my monitor
>>>>>> inflation stress tests on Linux-X64, MacOSX and Solaris-X64.
>>>>>>
>>>>>> Thanks, in advance, for any questions, comments or suggestions.
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>> On 3/24/19 9:57 AM, Daniel D. Daugherty wrote:
>>>>>>> Greetings,
>>>>>>>
>>>>>>> Welcome to the OpenJDK review thread for my port of Carsten's
>>>>>>> work on:
>>>>>>>
>>>>>>> JDK-8153224 Monitor deflation prolong safepoints
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8153224
>>>>>>>
>>>>>>> Here's a link to the OpenJDK wiki that describes my port:
>>>>>>>
>>>>>>> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation
>>>>>>>
>>>>>>>
>>>>>>> Here's the webrev URL:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/3-for-jdk13/
>>>>>>>
>>>>>>> Here's a link to Carsten's original webrev:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~cvarming/monitor_deflate_conc/0/
>>>>>>>
>>>>>>> Earlier versions of this patch have been through several rounds of
>>>>>>> preliminary review. Many thanks to Carsten, Coleen, Robbin, and
>>>>>>> Roman for their preliminary code review comments. A very special
>>>>>>> thanks to Robbin and Roman for building and testing the patch in
>>>>>>> their own environments (including specJBB2015).
>>>>>>>
>>>>>>> This version of the patch has been thru Mach5 tier[1-8] testing on
>>>>>>> Oracle's usual set of platforms. Earlier versions have been run
>>>>>>> through my stress kit on my Linux-X64 and Solaris-X64 servers
>>>>>>> (product, fastdebug, slowdebug).Earlier versions have run
>>>>>>> Kitchensink
>>>>>>> for 12 hours on MacOSX, Linux-X64 and Solaris-X64 (product,
>>>>>>> fastdebug
>>>>>>> and slowdebug). Earlier versions have run my monitor inflation
>>>>>>> stress
>>>>>>> tests for 12 hours on MacOSX, Linux-X64 and Solaris-X64 (product,
>>>>>>> fastdebug and slowdebug).
>>>>>>>
>>>>>>> All of the testing done on earlier versions will be redone on the
>>>>>>> latest version of the patch.
>>>>>>>
>>>>>>> Thanks, in advance, for any questions, comments or suggestions.
>>>>>>>
>>>>>>> Dan
>>>>>>>
>>>>>>> P.S.
>>>>>>> One subtest in gc/g1/humongousObjects/TestHumongousClassLoader.java
>>>>>>> is currently failing in -Xcomp mode on Win* only. I've been trying
>>>>>>> to characterize/analyze this failure for more than a week now. At
>>>>>>> this point I'm convinced that Async Monitor Deflation is
>>>>>>> aggravating
>>>>>>> an existing bug. However, I plan to have a better handle on that
>>>>>>> failure before these bits are pushed to the jdk/jdk repo.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>
-------------- next part --------------
Functional:
- Add MacroAssembler::inc_om_ref_count() to handle ObjectMonitor*
uses in C2 code safely.
- Near the end of C2 fast_unlock(), after the reacquire of the
ObjectMonitor to ensure proper succession, there is a race with
a deflated and recycled ObjectMonitor. Thanks to Robbin for the
MoCrazy test that revealed this race. Use inc_om_ref_count() to
close the race.
- Replace the deflated and recycled ObjectMonitor race handling
code in C2 fast_lock() with inc_om_ref_count() to close the race
without acquiring ownership and then dropping ownership. Thanks
to Robbin and Erik O for convincing me that there was still a
problem/race in the original fix.
- Update MacroAssembler::rtm_inflated_locking() to avoid races
with async monitor deflation by using inc_om_ref_count().
- Refactor ObjectMonitor::set_owner() and the direct owner field
settings into set_owner_from(2 forms), set_owner_from_BasicLock(),
and try_set_owner_from(). The new functions have guarantee()'s
for their expected old_value conditions and I've added more
trace level logging using 'monitorinflation+owner'.
- ObjectSynchronizer::prepend_block_to_lists() does not need
load_acquire() and release_store for g_block_list. Thanks
to David H. for catching that one.
- Simplified prepend_to_common() to use mark_next_loop() and
mark_list_head() directly.
- Simplified ObjectSynchronizer::om_release(), deflate_monitor_list(),
and deflate_monitor_list_using_JT() to take advantage of the
state associated with cur_mid_in_use and the fact that either
the list head is marked or mid is marked.
- In inflate()'s "CASE: stack-locked" block, set the allocation
state to Old after the object is updated to refer to the
ObjectMonitor. (Theoretical race fix)
- In inflate()'s "CASE: neutral" block, set the allocation state
to Old after the object is updated to refer to the ObjectMonitor.
Thanks to Robbin for the MoCrazy test that revealed this race.
- In ObjectSynchronizer::audit_and_print_stats(), change
g_om_population error to a warning.
- In ObjectSynchronizer::chk_global_in_use_list_and_count(), change
g_om_in_use_count error to a warning.
- rehn, eosterlund CR - add support for HandshakeAfterDeflateIdleMonitors
for platforms that don't have ObjectMonitor ref_count support
implemented in C2 fast_lock() and fast_unlock(). This includes a
new global wait list (g_wait_list) to hold deflated ObjectMonitors
until after a handshake/safepoint with all the JavaThreads.
Test update:
- None.
Collateral:
- Change 'jccb' -> 'jcc' and 'jmpb' -> 'jmp' as needed due to more
code in fast_unlock().
- Add '(int32_t)' cast to last movptr() in fast_lock(); it doesn't
matter if r10/obj is trashed at this point, but I wanted to be
consistent.
- Add ObjectMonitor::print_debug_style_on() to print a complete
ObjectMonitor like the debugger would do. It helps when debugging
on platforms where the debugger is not very functional with
release bits.
Cleanup:
- Add/update movptr(Address, literal) comments.
- Cleanup "fast-path" vs "fast path" and "slow-path" vs "slow path"
in X64 MacroAssembler locking functions. Also cleanup "Fast_Lock"
and "Fast_Unlock" to "fast_lock" and "fast_unlock".
- Drop clearing of ObjectMonitor's header/dmw field from the end
of ObjectMonitor::install_displaced_markword_in_object() since
it is already handled when the ObjectMonitor is deflated and
taken off the global free list.
- Clarify comment about ObjectMonitor's header/dmw field in
ObjectMonitor::install_displaced_markword_in_object().
- Misc comment additions/updates.
- coleenp CR - refactor async monitor deflation work from
ServiceThread::service_thread_entry() to
ObjectSynchronizer::deflate_idle_monitors_using_JT()
Temporary:
- I've moved -XX:[+-]CheckMonitorLists and all the supporting
code into a separate "debug" patch that I qpush when I need it.
Of course, that patch is not included in the RFR webrevs...
- ObjectMonitor::exit() has some logic that catches this error:
Non-balanced monitor enter/exit!
I have extra debugging code in there right now that will come
out after a few more complete test cycles have given me more
confidence that all the bug variants are squashed.
More information about the hotspot-runtime-dev
mailing list