Hi Peter, Thank you for fixing the code.
I updated webrev: http://cr.openjdk.java.net/~horii/8187033/webrev.02/ Regards, Ogata Peter Levart <[email protected]> wrote on 2017/09/04 17:45:37: > From: Peter Levart <[email protected]> > To: Kazunori Ogata <[email protected]> > Cc: core-libs-dev <[email protected]>, Hans Boehm > <[email protected]>, Aleksey Shipilev <[email protected]> > Date: 2017/09/04 17:45 > Subject: Re: RFR: 8187033: [PPC] Imporve performance of > ObjectStreamClass.getClassDataLayout() > > Hi Ogata, <snip> > If playing with mutable plain fields in multiple threads, it is > mandatory to read the plain field just once in program. Your implementation: > > 1196 ClassDataSlot[] getClassDataLayout() throws InvalidClassException { > 1197 // REMIND: synchronize instead of relying on volatile? > 1198 if (dataLayout == null) { > 1199 ClassDataSlot[] slots = getClassDataLayout0(); > 1200 VarHandle.fullFence(); > 1201 dataLayout = slots; > 1202 } > 1203 return dataLayout; > 1204 } > > reads dataLayout field twice (line 1198 and 1203). Those two reads may > reorder and 1st return non-null value, while the 2nd return previous > value - null. You should use a local variable to store the 1st read and > return the local variable at the end. Like: > > ClassDataSlot[] getClassDataLayout() throws InvalidClassException { > ClassDataSlot[] slots = dataLayout; > if (slots == null) { > ClassDataSlot[] slots = getClassDataLayout0(); > VarHandle.fullFence(); > dataLayout = slots; > } > return slots; > } > > > Regards, Peter >
