RFR(L) 8153224 Monitor deflation prolong safepoints (CR8/v2.08/11-for-jdk14)

[Daniel D. Daugherty]

Greetings,

I'm only going to jump in on one more single item here. This one is
more difficult than the last one since I have to do more snipping
to tie together the contexts here...


On 11/11/19 7:41 AM, Robbin Ehn wrote:
> Hi David,
>
> On 2019-11-11 11:52, David Holmes wrote:
>
>> For example you say:
>>
>>  > 242   jint l_ref_count = ref_count();
>>  > 243   ADIM_guarantee(l_ref_count > 0, "must be positive: 
>> l_ref_count=%d,
>>  > ref_count=%d", l_ref_count, ref_count());
>>  > Please use Atomic::load() in ref_count.
>>
<snip>
> Argubly above should be written as:
> jint l_ref_count = ref_count(); // Atomic::load()
> if (l_ref_count > 0) {
>     OrderAccess::loadload();
>     ADIM_guarantee(l_ref_count > 0, "must be positive: l_ref_count=%d,
>     ref_count=%d", l_ref_count, ref_count());
> }
>
> But since _ref_count could have been changed many times before the 
> second load I didn't see the point of printing the same value again.

Two things about this sub-thread:

- The reason for the second "ref_count=%d" is additional info
   in the case of a failure. If the l_ref_count value fails the
   check and the second printing shows a value that matches the
   condition, then I have one more tidbit of info about the race.
   And I get that information without looking at a core file in a
   debugger which doesn't always work.
- The above rewrite is not equivalent logic because this:

      if (l_ref_count > 0) {

   will prevent ADIM_guarantee(l_ref_count > 0, ...) from ever failing.

My plan is to update the ref_count() function to do an
Atomic::load() of the _ref_count field and I think that will
address the sub-thread to everyone's satisfaction.

Dan