[RESUMED] RFR: 8158946 - btree009 fails with assert(s > 0) failed: Bad size calculated

Thomas Schatzl thomas.schatzl at oracle.com
Tue Jun 28 13:09:30 UTC 2016


Hi,

On Mon, 2016-06-27 at 10:10 -0400, Derek White wrote:
> I'd like to split out the memory fence issue from the race fixed by
> this webrev. I think the fence issue may require more performance
> testing and several attempts to get something satisfactory.
> 
> New bug created: 
>     JDK-8160369 Memory fences needed around setting and reading
> object lengths.
> 
> How do reviewers feel about this patch to fix the initial race
> condition?

  looking at the 02 webrev:

- http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/gc
/shared/collectedHeap.inline.hpp.frames.html
105   // set the j.l.Class instance's oop_size field BEFORE setting the
header:

I would like to have the "why" answered here in this comment and not a
repetition of the code. I think something like: "Concurrent readers
expect that the size is set before the klass pointer."

Maybe the comment in lines 226/227 are more appropriate here?

- the "obj" parameter is cast to an oop four times in
CollectedHeap::post_allocation_setup_class. Could you add a local
variable?

- http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/oo
ps/instanceMirrorKlass.cpp.frames.html

Not sure if repeating the exit condition in line 59 makes sense.

- http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/oo
ps/oop.inline.hpp.frames.html

Maybe fix the comment in 261 to a proper sentence. (And possibly 262,
like "Oop size must be larger than zero but is %d")

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list