RFR(L) 8153224 Monitor deflation prolong safepoints (CR7/v2.07/10-for-jdk14)

Daniel D. Daugherty daniel.daugherty at oracle.com
Thu Oct 31 01:21:34 UTC 2019


This is just a checklist email to make sure that I addressed
everything in this particular review email...


On 10/29/19 4:20 PM, Daniel D. Daugherty wrote:
> Hi David,
>
> Sorry for the delay in replying to this email. I spent most of last week
> migrating my development environment from my dying MBP13 to a new MBP13.
> I think I have everything recovered, but only time will tell...
>
> More below...
>
>
> On 10/24/19 7:00 AM, David Holmes wrote:
>> Hi Dan,
>>
>> Final part of review - sorry for the delay.
>>
>> src/hotspot/share/runtime/objectMonitor.hpp
>>
>>  387   // For internal used by ObjectSynchronizer::monitors_iterate().
>>
>> s/used/use/
>
> Will fix that.

That comment was deleted by a change in CR8 so this is "done". :-)


>
>
>> ---
>>
>> src/hotspot/share/runtime/objectMonitor.inline.hpp
>>
>>  65   return OrderAccess::load_acquire(&_owner) == DEFLATER_MARKER;
>>
>> I query the need for load_acquire here.
>
> With all of the _owner field setting logic now encapsulated in the
> set_owner_from(), set_owner_from_BasicLock() and try_set_owner_from()
> functions, it is easy to see that all except for 
> set_owner_from_BasicLock()
> use cmpxchg() for the update. And with set_owner_from_BasicLock(), we
> already know that _owner != DEFLATER_MARKER.
>
> I agree and will change that line to:
>
>     65   return _owner == DEFLATER_MARKER;
>
> Update: Now I'm questioning making this change because of the possible
> changes to set_owner_from() below... Sigh...

Done.

All updates that set _owner to DEFLATER_MARKER and all updates
that set _owner from DEFLATER_MARKER are all done using a
cmpxchg() so I don't think we have an issue due to set_owner_from()
changes below.


>
>
>>
>>   71   assert(ref_count() == 0, "must be 0: ref_count=%d", ref_count());
>>   94     guarantee(ref_count() <= 0, "must be <= 0: ref_count=%d", 
>> ref_count());
>>
>> You don't want to read ref_count() twice else you may fail the check 
>> but report the expected value of 0.
>
> Agreed. I'll fix those to use a local so we report what we tested. I did
> that in another location that also reported the current value in the same
> error message. That helped with debugging some of the races.

Done in this file and in others.

Here's an example:

     jint l_ref_count = ref_count();
     guarantee(l_ref_count <= 0, "must be <= 0: l_ref_count=%d, 
ref_count=%d", l_ref_count, ref_count());


>
>>  122 // Set _owner field to new_value; current value must match 
>> old_value.
>>  123 inline void ObjectMonitor::set_owner_from(void* new_value, void* 
>> old_value) {
>>  124   void* prev = Atomic::cmpxchg(new_value, &_owner, old_value);
>>  125   ADIM_guarantee(prev == old_value, "unexpected prev owner=" 
>> INTPTR_FORMAT
>>
>> The use of cmpxchg seems a little strange here if you are asserting 
>> that when this is called _owner must equal old_value. That means you 
>> don't expect any race and if there is no race with another thread 
>> writing to _owner then you don't need the cmpxchg. A normal:
>>
>> if (_owner == old_value) {
>>    Atomic::store(&_owner, new_value);
>>    log(...);
>> } else {
>>    guarantee(false, " unexpected old owner ...");
>> }
>
> The two parameter version of set_owner_from() is only called from three
> places and we'll cover two of them here:
>
> src/hotspot/share/runtime/objectMonitor.cpp:
>
> 1041     if (AsyncDeflateIdleMonitors) {
> 1042       set_owner_from(NULL, Self);
> 1043     } else {
> 1044       OrderAccess::release_store(&_owner, (void*)NULL); // drop 
> the lock
> 1045       OrderAccess::storeload();                        // See if 
> we need to wake a successor
> 1046     }
>
> and:
>
> 1221   if (AsyncDeflateIdleMonitors) {
> 1222     set_owner_from(NULL, Self);
> 1223   } else {
> 1224     OrderAccess::release_store(&_owner, (void*)NULL);
> 1225     OrderAccess::fence();                               // ST 
> _owner vs LD in unpark()
> 1226   }
>
> So I've replaced the existing {release_store(), storeload()} combo for 
> one
> call site and the existing {release_store(), fence()} combo for the other
> call site with a cmpxchg(). I chose cmpxchg() for these reasons:
>
> 1) I wanted the same memory sync behavior at both call sites.
> 2) I wanted similar/same memory sync behavior as the original
>    code at those call sites.
> 3) I wanted the return value from cmpxchg() for my state machine
>    sanity check.
>
> I don't think that using 'Atomic::store(&_owner, new_value)' is the
> right choice for these two call sites.
>
> The last two parameter set_owner_from() is talked about in the
> next reply.
>
>
>> Similarly for the old_value1/old_valuie2 version.
>
> The three parameter version of set_owner_from() is only called from one
> place and the last two parameter version is called from the same place:
>
> src/hotspot/share/runtime/synchronizer.cpp:
>
> 1903       if (AsyncDeflateIdleMonitors) {
> 1904         m->set_owner_from(mark.locker(), NULL, DEFLATER_MARKER);
> 1905       } else {
> 1906         m->set_owner_from(mark.locker(), NULL);
> 1907       }
>
> The original code was:
>
> 1399       m->set_owner(mark.locker());
>
> The original set_owner() code was defined like this:
>
>   87 inline void ObjectMonitor::set_owner(void* owner) {
>   88   _owner = owner;
>   89 }
>
> So the original code didn't do any memory sync'ing at all and I've
> changed that to a cmpxchg() on both code paths. That appears to be
> overkill for that callsite...
>
> We're in ObjectSynchronizer::inflate(), in the "CASE: stack-locked"
> section of the code. We've gotten our ObjectMonitor from om_alloc()
> and are initializing a number of fields in the ObjectMonitor. The
> ObjectMonitor is not published until we do:
>
> 1916       object->release_set_mark(markWord::encode(m));
>
> So we don't need the memory sync'ing features of the cmpxchg() for
> either of the set_owner_from() calls and all that leaves is the
> state machine sanity check.
>
> I really like the state machine sanity check on the owner field but
> that's just because it came in handy when chasing the recent races.
> It would be easy to change the three parameter version of
> set_owner_from() to not do memory sync'ing, but still do the state
> machine sanity check.
>
> Update: Changing the three parameter version of set_owner_from()
> may impact the changes to owner_is_DEFLATER_MARKER() discussed
> above. Sigh...
> Update 2: Probably no impact because the three parameter version of
> set_owner_from() is only used before the ObjectMonitor is published
> and owner_is_DEFLATER_MARKER() is used after the ObjectMonitor has
> appeared on an in-use list.
>
> However, the two parameter version of set_owner_from() needs its
> memory sync'ing behavior for it's objectMonitor.cpp call sites so
> this call site would need something different.
>
> I'm not sure which solution I'm going to pick yet, but I definitely
> have to change something here since we don't need cmpxchg() at this
> call site. More thought is required.

Here's what I changed here:

   - Rename three param set_owner_from() to simply_set_owner_from() and
     drop cmpxchg() so the set of the _owner field to new_value happens
     without any memory sync.
   - Add two param simply_set_owner_from() that sets the _owner field to
     new_value without any memory sync.

and:

   - Update (monitorinflation, owner) log messages to provide more caller
     context.


>
>> This function does not aid clarity in my view, given that it replaces 
>> a simple assignment:
>>
>> // Set _owner field to self; current value must match basic_lock_p.
>> inline void ObjectMonitor::set_owner_from_BasicLock(Thread* self, 
>> void* basic_lock_p) {
>>   assert(self->is_lock_owned((address)basic_lock_p), "self=" 
>> INTPTR_FORMAT
>>          " must own basic_lock_p=" INTPTR_FORMAT, p2i(self), 
>> p2i(basic_lock_p));
>>   void* prev = _owner;
>>   ADIM_guarantee(prev == basic_lock_p, "unexpected prev owner=" 
>> INTPTR_FORMAT
>>                  ", expected=" INTPTR_FORMAT, p2i(prev), 
>> p2i(basic_lock_p));
>>   // Non-null owner field to non-null owner field is safe without
>>   // cmpxchg() as long as all readers can tolerate either flavor.
>>   _owner = self;
>>   log_trace(monitorinflation, owner)("mid=" INTPTR_FORMAT ", prev="
>>                                      INTPTR_FORMAT ", new=" 
>> INTPTR_FORMAT,
>>                                      p2i(this), p2i(prev), p2i(self));
>> }
>>
>> the places where this is used have already read _owner and passed it 
>> in as basic_lock_p; and they have already checked 
>> self->is_locked_owned. The only useful thing here is potentially the 
>> log statement but even that is lacking as noone will be able to 
>> readily tell that this was a basic_lock conversion. The log statement 
>> here (and elsewhere) should give an indication of the nature of the 
>> update to owner. But I'd rather see this method go and just add a 
>> context specific log statement at each of the places where we morph 
>> the basic_lock to the thread.
>
> So encapsulate some of the sites that set the _owner field, but not all?
> Once I decided to go down the encapsulation road, I figured I should
> encapsulate them all for consistency. I guess not. I'm thinking that
> Coleen may not be happy with this inconsistency in encapsulation.
>
> So the other thing that will be lost is the explicit state machine sanity
> check:
>
>   void* prev = _owner;
>   ADIM_guarantee(prev == basic_lock_p, "unexpected prev owner=" 
> INTPTR_FORMAT
>                  ", expected=" INTPTR_FORMAT, p2i(prev), 
> p2i(basic_lock_p));
>
> This check will detect if the _owner field unexpected changes from the 
> BasicLock*
> address to something else due to a race. The particular race I was 
> interested in
> was whether more one thread could be trying to morph the BasicLock* 
> into the
> JavaThread* at the same time. I have a vague memory of this being 
> possible due
> to biased locking/deoptimization work, but I could be wrong. I never 
> saw this
> guarantee() fire during any of my testing, stress or otherwise. And my 
> memory
> of one thread morphing another thread's BasicLock* into a JavaThread* 
> is a
> very old one; things could easily have changed...
>
> I will look at undoing the encapsulation with 
> set_owner_from_BasicLock(). I'll
> also look at updating the log messages that I added in this round to 
> be more
> helpful.

Here's what I ended up doing here:

   - Rename set_owner_from_BasicLock() to simply_set_owner_from_BasicLock(),
     drop "assert(is_lock_owned(basic_lock_p))" since all the callers
     already check that.
   - Update (monitorinflation, owner) log messages to provide more caller
     context.

I agree that the first assert() in the function was redundant and I
have removed it. I decided to keep the encapsulation because I want to
keep the state machine guarantee() check and the logging in one place
rather than duplicate those two multi-line pieces at all the call sites.


>
>
>>  199 // The decrement only needs to be MO_ACQ_REL since the reference
>>  200   // counter is volatile.
>>  201   Atomic::dec(&_ref_count);
>>
>> volatile is irrelevant with regards to memory ordering as it is a 
>> compiler annotation. And you haven't specified any memory order value 
>> so the default is conservative ie. implied full fence. (I see the 
>> same incorrect comment is in threadSMR.cpp!)
>
> I got that wording from threadSMR.cpp and Erik O. confirmed my use of 
> that
> wording previously. I'll chase it down with Erik and get back to you.

I still have to ping Erik O. about this.


>
>
>> 208   // The increment needs to be MO_SEQ_CST so that the reference
>>  209   // counter update is seen as soon as possible in a race with the
>>  210   // async deflation protocol.
>>  211   Atomic::inc(&_ref_count);
>>
>> Ditto you haven't specified any ordering - and inc() and dec() will 
>> have the same default.
>
> And again, I'll have to chase this down with Erik O. and get back to you.

I still have to ping Erik O. about this.


>
>>
>>  218   return OrderAccess::load_acquire(&_ref_count);
>>
>> Again I query the use of load_acquire here.
>
> For more context:
>
>  217 inline jint ObjectMonitor::ref_count() const {
>  218   return OrderAccess::load_acquire(&_ref_count);
>  219 }
>
> I concur that it looks like this can change to:
>
>   218   return _ref_count;
>
> because all the updates to the _ref_count field are done via:
> Atomic::inc(), Atomic::dec() or cmpxchg().

Done.


>
> Looking at the history of the code, I did have a race where I came to
> the conclusion that the load_acquire() was needed. I'll have to go back
> to that race's analysis and double check my thinking.

Still have to do the research...


>
>> ---
>>
>> src/hotspot/share/runtime/objectMonitor.cpp
>>
>>
>>  267   if (AsyncDeflateIdleMonitors &&
>>  268       try_set_owner_from(Self, DEFLATER_MARKER) == 
>> DEFLATER_MARKER) {
>
> For more context, we are in:
>
>  241 void ObjectMonitor::enter(TRAPS) {
>
>
>> I don't see why you need to call try_set_owner_from again here as 
>> "cur" will already be DEFLATER_MARKER from the previous try_set_owner.
>
> I assume the previous try_set_owner() call you mean is this one:
>
>  248   void* cur = try_set_owner_from(Self, NULL);
>
> This first try_set_owner() is for the most common case of no owner.
>
> The second try_set_owner() call is for a different condition than the 
> first:
>
>  268       try_set_owner_from(Self, DEFLATER_MARKER) == 
> DEFLATER_MARKER) {
>
> L248 is trying to change the _owner field from NULL -> 'Self'.
> L268 is trying to change the _owner field from DEFLATER_MARKER to 'Self'.
>
> If the try_set_owner() call on L248 fails, 'cur' can be several possible
> values:
>
>   - the calling thread (recursive enter is handled on L254-7)
>   - other owning thread value (BasicLock* or Thread*)
>   - DEFLATER_MARKER
>
>
>> Further, I don't see how installing self as the _owner here is valid 
>> and means you acquired the monitor, as the fact it was 
>> DEFLATER_MARKER means it is still being deflated by another thread 
>> doesn't it ???
>
> I guess the comment after L268 didn't work for you:
>
>  269     // The deflation protocol finished the first part (setting 
> owner),
>  270     // but it failed the second part (making ref_count negative) and
>  271     // bailed. Or the ObjectMonitor was async deflated and reused.
>
> It means that the deflater thread was racing with this enter and
> managed to set the owner field to DEFLATER_MARKER as the first step
> in the deflation protocol. Our entering thread actually won the race
> when it managed to set the ref_count to a positive value as part of
> the ObjectMonitorHandle stuff done in the inflate() call that preceded
> the enter() call. However, the deflater thread hasn't realized that it
> lost the race yet and hasn't restored the owner field back to NULL.
>
> This particular race is covered by this subsection in the wiki:
>
> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation#AsyncMonitorDeflation-T-enterWinsByA-B-A 
>
>
> If the entering thread succeeds in the try_set_owner_from() call on L268,
> it will indeed be the legitimate owner of the ObjectMonitor. The 
> deflating
> thread will eventually realize that it lost the race and when it tries to
> restore the owner field from DEFLATER_MARKER -> NULL, it will see that 
> the
> owner field is no longer DEFLATER_MARKER and just bail out.
>
>
>>  303 assert(AsyncDeflateIdleMonitors || this->object() != NULL, 
>> "invariant");
>>
>> I don't see how AsyncDeflateIdleMonitors permits a NULL object() at 
>> this point given we have hit the contended path!
>
> Good question! I think that assertion adjustment is from before we had
> ref_counts and the ref_count now prevents us from getting as far as
> clearing the _object field in the async deflation process. I'll restore
> that back to the original baseline assert.

Deleted the "(AsyncDeflateIdleMonitors ||" part.


>
>>  508 } else {
>>  509     ss->print("owner=" INTPTR_FORMAT, NULL);
>>
>> wouldn't you want to report that it is deflating in this case?
>
> More context here:
>
>  506   } else if (_owner != DEFLATER_MARKER) {
>  507     ss->print("owner=" INTPTR_FORMAT, p2i(_owner));
>  508   } else {
>  509     ss->print("owner=" INTPTR_FORMAT, NULL);
>  510   }
>
> so we intentionally print NULL instead of DEFLATER_MARKER here because
> the DEFLATER_MARKER value doesn't make the is_busy() function return
> true. Unfortunately, is_busy() is over in objectMonitor.hpp:
>
>  252   intptr_t is_busy() const {
>  253     // TODO-FIXME: assert _owner == null implies _recursions = 0
>  254     // We do not include _ref_count in the is_busy() check because
>  255     // _ref_count is for indicating that the ObjectMonitor* is in
>  256     // use which is orthogonal to whether the ObjectMonitor itself
>  257     // is in use for a locking operation.
>  258     intptr_t ret_code = _contentions | _waiters | intptr_t(_cxq) 
> | intptr_t(_EntryList);
>  259     if (!AsyncDeflateIdleMonitors) {
>  260       ret_code |= intptr_t(_owner);
>  261     } else {
>  262       if (_owner != DEFLATER_MARKER) {
>  263         ret_code |= intptr_t(_owner);
>  264       }
>  265     }
>  266     return ret_code;
>  267   }
>
> I'll add comment between L508 and L509 to explain why we are ignoring
> DEFLATER_MARKER. Something like:
>
>     // We report NULL instead of DEFLATER_MARKER here because is_busy()
>     // ignores DEFLATER_MARKER values.

Done.


>
>>  533 if (AsyncDeflateIdleMonitors &&
>>  534       try_set_owner_from(Self, DEFLATER_MARKER) == 
>> DEFLATER_MARKER) {
>>
>>  660     if (AsyncDeflateIdleMonitors &&
>>  661         try_set_owner_from(Self, DEFLATER_MARKER) == 
>> DEFLATER_MARKER) {
>>
>>  792     if (AsyncDeflateIdleMonitors &&
>>  793         try_set_owner_from(Self, DEFLATER_MARKER) == 
>> DEFLATER_MARKER) {
>>
>> Same query as above - line 267 - how does this imply a free monitor 
>> ready for use ??
>
> Same answer as above. And I have the same/similar comment explaining what
> happened if we succeed in the try_set_owner_from() call for each of the
> above sites.
>
>
>> 1005       tty->print_cr("ERROR: ObjectMonitor::exit(): thread=" 
>> INTPTR_FORMAT
>> 1006                     " is exiting an ObjectMonitor it does not 
>> own.",
>> 1007                     p2i(THREAD));
>> 1008       tty->print_cr("The imbalance is possibly caused by JNI 
>> locking.");
>> 1009       print_debug_style_on(tty);
>>
>> Leftover debug output?
>
> Yes, as I mentioned in the CR6-to-CR7-changes attachment to the code
> review email for this round:
>
> > Temporary:
> > <snip>
> >   - 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.
>
> I haven't seen this issue pop up during the entire CR7 testing round.
> Solaris-X64 is the last platform to have a sighting prior to CR7 and
> that was after 7951 iterations of "MoCrazy 1024". Solaris-X64 is
> currently > 15000 iterations and 11+ days... I'll leave it running
> until I'm ready to start CR8 testing...
>
> So some of the debug output will go away. Other parts will return to
> being non-release bits assert() and possibly some log_error() output.

I'll do this debug code deletion after I get thru the
CR8 test cycle. This could be useful if I broke something
in CR8 since there are non-trivial changes...


>
>> If you actually want a print here it should be a warning or some 
>> other logging operation. (Aside: this happens more often with JIT 
>> bugs than actual JNI locking :) ).
>
> Yup. I've actually never seen it with JNI locking during this project.
>
> It appears that the one or two tests we had that explicitly exercised
> this part of JNI locking are no longer run as part of Mach5 Tier[1-8].
>
> At one point, Jerry and I discussed adding a diagnostic option to
> enable this check in 'release' bits. I still have the note on my
> whiteboard from Colorado:
>
>     Jerry -> AssertOnMismatchedMonitor
>           -> FailOnMismatchedMonitor
>
> as the two choices to think about. It's probably going to be really
> hard to erase that part of the whiteboard... That note has been there
> for quite a while...
>
>
>>
>> 1039     // On SPARC that requires MEMBAR #loadstore|#storestore.
>> 1040     // But of course in TSO #loadstore|#storestore is not required.
>>
>> Old comments no longer needed or relevant given the code uses 
>> OrderAccess regardless of platform.
>
> Always happy to delete or correct stale comments. Will delete.

Done.


>
>> ---
>>
>> That's it!
>
> Thanks for such a thorough review!! I greatly appreciate you taking
> the time for a crawl thru review!!
>
>
> I'm going to return to making changes for CR8. At this point, I've
> (tried to) address for CR7:
>
> - 3 OpenJDK review emails from David H.
> - 2 private review emails from Robbin
> - 1 private review email from Per
>
> If I've missed a review email from someone I apologize and please
> resend it; email may have been lost during my transition from my dying
> MBP13 to my new MBP13.
>
> Dan

Okay. I think I've made all the changes from David H's part 3 of 3
code review email.

Please let me know if I'm missed anything.

Dan


>
>
>>
>> Thanks,
>> David
>> -----
>>
>>
>> On 18/10/2019 7:50 am, Daniel D. Daugherty wrote:
>>> 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
>>>
> <trimmed older review invites>



More information about the hotspot-runtime-dev mailing list