Hi Aleksey, On Tue, Nov 4, 2014 at 11:55 AM, Aleksey Shipilev <aleksey.shipi...@oracle.com> wrote: > On 04.11.2014 20:53, Martin Buchholz wrote: >> On Tue, Nov 4, 2014 at 2:13 AM, Paul Sandoz <paul.san...@oracle.com> wrote: >>> On Nov 3, 2014, at 11:18 PM, Martin Buchholz <marti...@google.com> wrote: > If you do this: > >> // Generic info repository; lazily initialized >> - private transient ClassRepository genericInfo; >> + private volatile transient ClassRepository genericInfo; > > ...you don't need to do this for correctness (but may do for performance > -- avoidable volatile read for ARM/PPC):
It's just reflex to always read volatiles into locals - never hurts, except the line-of-code count. >> // accessor for generic info repository >> private ClassRepository getGenericInfo() { >> + ClassRepository genericInfo = this.genericInfo; >> // lazily initialize repository if necessary >> if (genericInfo == null) { >> // create and cache generic info repository >> genericInfo = ClassRepository.make(getGenericSignature(), >> getFactory()); >> + this.genericInfo = genericInfo; >> } >> return genericInfo; //return cached repository >> } >> > > Also, I wonder if we need a singleton ClassRepository instance, or we > can tolerate two different ClassRepository-ies returned under concurrent > init. Yeah, me too. But the existing code has this behavior as well. One could fix it if needed with double-checked-locking or whatever our fancy new volatile unsafe-less cas ends up looking like.