----- Original message ----- > On 10/27/2011 1:17 PM, Peter wrote: > > That's exactly what the original implementers needed to do, make those > > fields > > volatile. > > > > They're private implementation fields though. > > Okay, but are there any usage patterns where we could add the use of a > volatile? > The JMM says that multiple non-volatile fields can be made visible by a >single > volatile write and then another thread making a volatile read. So if we are > adding properties, we should do a volatile write after that, and then if there > is a place before the use of permissioncollection, by another thread, that we > can force a volatile read of the same volatile field, then that should fix the > visibility. It's not pretty...
Interesting, read it, mutate it, write it back, didn't think of that. You're right it's not pretty. > > Should we create an issue in the JDK bugzilla? Yes. Actually, I don't think they should use any synchronisation, this can be provided by a wrapper class similar to collections. Cheers, Peter. > > Gregg > > > > > Trouble is, none of these old jvm homogenous PermissionCollection's have > > been > > exposed to any more than single threads before and the last thing I want to > > do > > is reimplement them. They're supposed to be thread safe but many have > > visibility issues. > > > > Considering java security policy is a occassional write, multi read, it > > should > > be simple to make it scale very well, using immutability and concurrency > > utils. There's just some legacy cruft that spoils it a little. > > > > I guess I could make a wrapper class that uses volatile and write replace, > > but then if it changes you still have to replace the underlying > > PermissionCollection, and still wear the synchronisation cost. > > > > Cheers, > > > > Peter. > > > > Cheers, > > > > Peter. > > > > ----- Original message ----- > > > What about a volatile as the visibility control? Write after update, read > > > before access? It would at least expose the changes to other threads, not > > > be a lock, and represent a fairly limited overhead on most hardware. > > > > > > Gregg > > > > > > On 10/27/2011 8:55 AM, Peter wrote: > > > > The problem: > > > > > > > > Stale references allowed and noted in comments: > > > > > > > > java.security.Permissions > > > > java.security.BasicPermissions.BasicPermissionCollection > > > > > > > > The stale reference in Permissions is an AllPermission object - an > > > > optimisation. If a thread doesn't see the current value, it just checks > > > > the internal Map, which is synchronised, no biggy. > > > > > > > > Problem is, Permissions is a heterogenous PermissionCollection, it > > > > contains a Map, synchronzed thread access, which prevents a similar > > > > optimisation in the homogenous BasicPermissionCollection from being seen > > > > in the stale state. > > > > > > > > Every ProtectionDomain has its own Permissions and each Permission class > > > > type has it's own unique PermissionCollection shared with all others > > > > with > > > > the same type for a ProtectionDomain. > > > > > > > > I replaced Permissions with a class called ConcurrentPermissions that > > > > uses > > > > a ConcurrentMap > > > > > > > > Trouble is BasicPermissionCollection is no longer protected by > > > > synchronization in Permissions. BasicPermissionCollection now exposed > > > > to > > > > multiple threads has a stale reference optimisation for wildcard * > > > > permissions. > > > > > > > > What happens in my concurrent policy implementation is the Permission > > > > isn't > > > > necessarily found in the BasicPermissionCollection by a second thread, > > > > so > > > > it checks the PermissionGrants (immutable objects that contain data from > > > > policy files or dynamic grants) again and adds all the permissions to > > > > BasicPermissionCollection again. So it doesn't fail, but it doesn't > > > > scale well with contention, because you've still got the synchronisation > > > > bottleneck, can't see the Permission and have to process again, wasting > > > > resources, on the second occassion. > > > > > > > > Problem is, BasicPermissionCollection is the bread and butter > > > > PermissionCollection implementation many Permission classes use. > > > > > > > > Now you have to remember, these classes were designed well before > > > > concurrency was ever a consideration. Nowadays these classes would be > > > > immutable, since policy's don't change much, they're mostly read access. > > > > > > > > But I can't change it because many are part of the decision process. > > > > > > > > Now I could put a synchronized wrapper PermissionCollection class around > > > > these things, which fixes the bug, creating long lived objects that live > > > > on the heap and will likely cause L2 cache misses or contended locks. > > > > > > > > How about something different? > > > > > > > > Create the PermissionCollection's on demand, then discard immediately > > > > after > > > > use. The Permission objects themselves are long lived immutable > > > > objects. > > > > > > > > Why? > > > > > > > > It'll be used only by one thread, so the jvm will optimise out the > > > > synchronised locks. > > > > > > > > The object will be created on the threads local memory stack, instead of > > > > the heap and die in the young generation, so it doesn't incur gc heap > > > > generation movements or memory heap copy to cpu cache stalls. > > > > > > > > But what about single thread applications or those with few threads and > > > > little contention? They would run slower, although object allocation > > > > costs aren't as bad as people think, say 10 to 20 cpu cycles compared to > > > > 200 for a cache miss, or worse for a contended lock. > > > > > > > > Pattern matching of strings is the most expensive computation of most > > > > permission decisions and has to be repeated for every ProtectionDomain > > > > on > > > > the call stack for each thread, the impact on single core machines won't > > > > be much. I can test for that, but not the high end stuff. > > > > > > > > Arrghh decisions! Not enough test hardware. > > > > > > > > Cheers, > > > > > > > > Peter. > > > > > > > > > > > >
