RFR: 8199739: Use HeapAccess when loading oops from static fields in javaClasses.cpp (original) (raw)
Kim Barrett kim.barrett at oracle.com
Sat Mar 17 18:41:47 UTC 2018
- Previous message: RFR: 8199739: Use HeapAccess when loading oops from static fields in javaClasses.cpp
- Next message: RFR: 8199739: Use HeapAccess when loading oops from static fields in javaClasses.cpp
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On Mar 17, 2018, at 6:39 AM, Roman Kennke <rkennke at redhat.com> wrote:
Am 17.03.2018 um 05:11 schrieb Kim Barrett:
On Mar 16, 2018, at 10:39 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
Hi all, Please review this patch to use HeapAccess<>::oopload instead of oopDesc::loaddecodeheapoop when loading oops from static fields in javaClasses.cpp: http://cr.openjdk.java.net/~stefank/8199739/webrev.01/ https://bugs.openjdk.java.net/browse/JDK-8199739 It's necessary to use HeapAccess<>::oopload to inject load barriers for GCs that need them. Thanks, StefanK ------------------------------------------------------------------------------ src/hotspot/share/classfile/javaClasses.cpp. 1870 address addr = ik->staticfieldaddr(staticunassignedstacktraceoffset); 1871 return HeapAccess<>::oopload((HeapWord*)addr); I'm not sure this is sufficient. Isn't staticfieldaddr just fundamentally broken for Shenandoah? It is not totally broken (I think addr() calls resolve() which kinda does what we need),
Current implementation of static_field_addr is:
address InstanceKlass::static_field_addr(int offset) { assert(offset >= InstanceMirrorKlass::offset_of_static_fields(), "has already been adjusted"); return (address)(offset + cast_from_oop(java_mirror())); }
java_mirror() calls OopHandle::resolve(), which presently doesn’t do anything wrto Access (has OopHandle not yet been Accessorized? Or is there some reason why it’s okay as is that I’m not thinking of right now?) Even if it did use Access, I would expect for Shenandoah the access would just be a dereference of the oop* in the OopHandle, and would not chase the Brooks pointer, since we’re not accessing fields in the mirror object at that point.
but the following would be perfect:
but I think such an access needs to be HeapAccess<>::loadat(mirror, offset) Thank you, Roman
- Previous message: RFR: 8199739: Use HeapAccess when loading oops from static fields in javaClasses.cpp
- Next message: RFR: 8199739: Use HeapAccess when loading oops from static fields in javaClasses.cpp
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]