Thanks for that salient comment. Perhaps someone can give us the right pattern for no lock Singleton initialization.
On May 25, 2010 8:49 PM, "Paul Cowan" <[email protected]> wrote: > On 26/05/2010 1:15 PM, Daniel Ploeg wrote: >>>> this is a lot of synchronization for what ends up being on the fast path of every query, perhaps there is a non synchronized manner in which we can accomplish this? the use of volatile might help here >> >> thanks Ryan. I think I might use AtomicReference instead - can simplify even further this way. I'll upload another patch a bit later. > > Sorry, no real stake in this review but just a curious third-party: is > there any need for even the AtomicReference? > > Making 'delegate' volatile would completely remove the need for the > synchronization in getDelegate(), and the lock on injectEdge(). The only > exception is that without synchronization, the null-check-and-default in > getDelegate() would be subject to race conditions. Given the desire to > reduce synchronization, and the fact that delegate is assigned to > default at construction time, I'd humbly suggest that the "if (delegate > == null)" be removed from getDelegate(), and instead change injectEdge() > to reject null values (or call reset() if you want to keep the effective > existing semantics that injectEdge(null) returns to a > DefaultEnvironmentEdge). > > Then the ReentrantLock can be removed, no synchronization is required, > and one volatile variable is all that's needed. > > (Incidentally, I'm not sure what the ReentrantLock is actually FOR -- > reference assignment is atomic in Java, so it's not protecting against > any race conditions; all it could be useful for is a happens-before > relationship with the get, but the two are using different locks -- the > synchronized get does not have a happens-before relationship with the > unlock. Either both should be synchronized, or both use the lock -- as > it is, the lock buys you nothing. Mind you with a simple volatile then > the happens-before relationship is correct anyway). > > Cheers, > > Paul
