RFR: 8196083: Avoid locking in OopStorage::release

[Erik Österlund]

Hi David,

This is starting to go a bit off-topic for this thread. But here goes...

On 2018-02-04 13:55, David Holmes wrote:
> On 4/02/2018 10:38 PM, Erik Osterlund wrote:
>> Hi Kim,
>>
>> Looks complicated but good.
>>
>> It would be great in the future if the deadlock detection system 
>> could be improved to not trigger such false positives that make us 
>> implement tricky lock-free code to dodge the obviously false positive 
>> deadlock assert. But I suppose that is out of scope for this.
>
> It isn't a deadlock-detection system it is a deadlock prevention 
> system. If you honour the lock rankings then you can't get deadlocks. 
> If you don't honour the lock rankings then you may get deadlocks. 
> There isn't sufficient information in the ranking alone to know for 
> sure whether you will or not.

Okay. I guess I should have called it a potential deadlock situation 
detection system. But it does not prevent deadlocks - that is up to us. 
And since the checking is dynamic, we are never guaranteed not to get 
deadlocks.

> If the deadlock possibility is so obviously not actually possible then 
> that could be captured somehow for the specific locks involved. But 
> I'm not aware of any tools we have that actually help us track what 
> locks may concurrently be acquired - if we did then we would not need 
> rank-based deadlock prevention checks.

What I had in mind is something along the lines of dynamically 
constructing a global partial ordering of the locks as they are 
acquired, and to verify the global partial ordering is consistent and 
not violated. That would be like a more precise version of the manually 
constructed ordering we have today, and save us the trouble of doing 
this manual picking of a number X, giving it a silly name nobody 
understands like "leaf + 3", where leaf is not actually a leaf at all - 
that's for "special", oh wait no there are more special lock ranks than 
special. And then as testing is run, either manually shuffling the ranks 
around to reflect the actual partial ordering the code adheres to, or 
rewriting the code to be lock-free in fear of getting intermittent false 
positive asserts triggered in testing after moving ranks around (despite 
every failing test run actually strictly conforming to a global partial 
ordering that was just not reflected accurately by the numbers we picked).

With such an automatic solution, we could also get a better picture of 
the interactions between the locks when adding a new lock by printing 
the actual partial ordering of the locks that was found at runtime, 
instead of trying to figure out which other relevant locks the "+ 3" in 
"leaf + 3" referred to.

Of course, this is just an idea for the bright future where a magical 
system can do lock ordering consistency checks automagically without us 
resorting to complicated lock-free solutions for code that never 
violates global lock ordering consistency (which is what the system was 
designed to detect), because it is found easier to write a lock-free 
solution to the problem at hand than to figure out how best to shuffle 
the ranks around to capture the actual partial ordering the locks 
consistently conform to. But for now, since we do not have such a 
system, and its future existence is merely hypothetical, I am okay with 
the proposed lock-free solution instead.

Thanks,
/Erik

> David
> -----
>
>> Thanks,
>> /Erik
>>
>>> On 3 Feb 2018, at 01:35, Kim Barrett <kim.barrett at oracle.com> wrote:
>>>
>>> Please review this change to the OopStorage::release operations to
>>> eliminate their use of locks.  Rather than directly performing the
>>> _allocate_list updates when the block containing the entries being
>>> released undergoes a state transition (full to not-full, not-full to
>>> empty), we instead record the occurrence of the transition. This
>>> recording is performed via a lock-free push of the block onto a list
>>> of such deferred updates, if the block is not already present in the
>>> list.  Update requests are processed by later allocate and
>>> delete_empty_block operations.
>>>
>>> Also backed out the JDK-8195979 lock rank changes for the JNI mutexes.
>>> Those are no longer required to nested lock rank ordering errors.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8196083
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~kbarrett/8196083/open.00/
>>>
>>> Testing:
>>> Reproducer from JDK-8195979.
>>> Mach5 {hs,jdk}-tier{1,2,3}
>>>
>>