Request for reviews (S): 7196242: vm/mlvm/indy/stress/java/loopsAndThreads crashed
john.r.rose at oracle.com
Mon Sep 10 17:06:12 PDT 2012
Locking on the CP.lock should be fast enough and will scale better than a global lock. It is also more robustly correct than CAS hacking. (Mea culpa for that one.). Thanks for the fix; reviewed.
-- John (on my iPhone)
On Sep 10, 2012, at 4:10 PM, Vitaly Davidovich <vitalyd at gmail.com> wrote:
> // Writers must store all other values before f1. 266 // Readers must test
> f1 first for non-null before reading other fields. 267 // Competing writers
> must acquire exclusive access on the first 268 // write, to flags, using a
> compare/exchange. 269 // A losing writer to flags must spin until the
> winner writes f1, 270 // so that when he returns, he can use the linked
> cache entry.
> OK maybe I'm just not seeing the spin loop or the cas in the webrev for
> cpCache.cpp ...
> I had assumed the lock impl would be fast when uncontended (one word CAS or
> similar) but I take your point.
> Sent from my phone
> On Sep 10, 2012 7:02 PM, "Christian Thalinger" <
> christian.thalinger at oracle.com> wrote:
>> On Sep 10, 2012, at 3:53 PM, Vitaly Davidovich <vitalyd at gmail.com> wrote:
>>> Hi Chris,
>>> Perhaps the comment above where you now take out a lock should be
>> updated since it's still talking about the CAS race?
>> No. These are two different problems.
>>> Also, curious why the CAS with immediate yield() was used before - seems
>> like that doesn't actually buy much since a losing thread may then
>> immediately lose CPU time and have to wait for scheduling to put it back on
>> (I.e. a lock seems more efficient anyway here :)).
>> But that's only in a race. When there is only one thread the current
>> approach might be faster. It depends on how fast the lock implementation
>> -- Chris
>>> Sent from my phone
>>> On Sep 10, 2012 5:30 PM, "Christian Thalinger" <
>> christian.thalinger at oracle.com> wrote:
>>> 29 lines changed: 13 ins; 10 del; 6 mod; 2239 unchg
>>> 7196242: vm/mlvm/indy/stress/java/loopsAndThreads crashed
>>> The current code in ConstantPoolCacheEntry::set_method_handle_common uses
>>> a CAS to find out who's the winning thread in the race about linking an
>>> invokehandle/invokedynamic call site. The winning thread does the
>>> while the other threads are waiting in a loop until the winning thread
>>> finishes. The waiting threads enter the Patching_lock and call os::yield
>>> to give the winning thread more CPU time.
>>> Unfortunately the implementation of os::yield on Solaris uses the
>>> Threads_lock and we hit this assert:
>>> # fatal error: acquiring lock Threads_lock/15 out of order with lock
>> Patching_lock/1 -- possible deadlock
>>> Even worse, the CAS might fail spuriously and we don't have a winning
>>> thread because we don't loop around the CAS. This may lead to hangs.
More information about the hotspot-dev