Hi Martin, On 04.11.2014 23:11, Martin Buchholz wrote: > 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.
No objection here :) Current JDK 9 code also does this. I think we can make a partial backport of JDK-8016236 to jdk7, since this volatile construction fixes the obvious bug, and the fix was tested already in JDK 8 and 9. Can anyone from Core Libraries team attend to this? >> 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. All right, ClassRepository seems to be an innocuous idempotent cache. Do not need double-checked locking here. -Aleksey.
