6912621: iCMS: Error: assert(_markBitMap.isMarked(addr + 1),"Missing Printezis bit?")
Summary: Fix block_size_if_printezis_bits() so it does not expect the bits, only uses them when available. Fix block_size_no_stall() so it does not stall when the bits are missing such cases, letting the caller deal with zero size returns. Constant pool cache oops do not need to be unparsable or conc_unsafe after their klass pointer is installed. Some cosmetic clean-ups and some assertion checking for conc-usafety which, in the presence of class file redefinition, has no a-priori time boundedness, so all GCs must be able to safely deal with putatively conc-unsafe objects in a stop-world pause.
Reviewed-by: jmasa, johnc
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -331,7 +331,7 @@
length, CHECK_(nullHandle));
constantPoolOop constant_pool =
oopFactory::new_constantPool(length,
- methodOopDesc::IsSafeConc,
+ oopDesc::IsSafeConc,
CHECK_(nullHandle));
constantPoolHandle cp (THREAD, constant_pool);
@@ -1929,10 +1929,9 @@
}
// All sizing information for a methodOop is finally available, now create it
- methodOop m_oop = oopFactory::new_method(
- code_length, access_flags, linenumber_table_length,
- total_lvt_length, checked_exceptions_length,
- methodOopDesc::IsSafeConc, CHECK_(nullHandle));
+ methodOop m_oop = oopFactory::new_method(code_length, access_flags, linenumber_table_length,
+ total_lvt_length, checked_exceptions_length,
+ oopDesc::IsSafeConc, CHECK_(nullHandle));
methodHandle m (THREAD, m_oop);
ClassLoadingService::add_class_method_size(m_oop->size()*HeapWordSize);
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -1040,9 +1040,10 @@
} else {
// must read from what 'p' points to in each loop.
klassOop k = ((volatile oopDesc*)p)->klass_or_null();
- if (k != NULL &&
- ((oopDesc*)p)->is_parsable() &&
- ((oopDesc*)p)->is_conc_safe()) {
+ // We trust the size of any object that has a non-NULL
+ // klass and (for those in the perm gen) is parsable
+ // -- irrespective of its conc_safe-ty.
+ if (k != NULL && ((oopDesc*)p)->is_parsable()) {
assert(k->is_oop(), "Should really be klass oop.");
oop o = (oop)p;
assert(o->is_oop(), "Should be an oop");
@@ -1051,6 +1052,7 @@
assert(res != 0, "Block size should not be 0");
return res;
} else {
+ // May return 0 if P-bits not present.
return c->block_size_if_printezis_bits(p);
}
}
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -6360,18 +6360,16 @@
// A variant of the above (block_size_using_printezis_bits()) except
// that we return 0 if the P-bits are not yet set.
size_t CMSCollector::block_size_if_printezis_bits(HeapWord* addr) const {
- if (_markBitMap.isMarked(addr)) {
- assert(_markBitMap.isMarked(addr + 1), "Missing Printezis bit?");
+ if (_markBitMap.isMarked(addr + 1)) {
+ assert(_markBitMap.isMarked(addr), "P-bit can be set only for marked objects");
HeapWord* nextOneAddr = _markBitMap.getNextMarkedWordAddress(addr + 2);
size_t size = pointer_delta(nextOneAddr + 1, addr);
assert(size == CompactibleFreeListSpace::adjustObjectSize(size),
"alignment problem");
assert(size >= 3, "Necessary for Printezis marks to work");
return size;
- } else {
- assert(!_markBitMap.isMarked(addr + 1), "Bit map inconsistency?");
- return 0;
- }
+ }
+ return 0;
}
HeapWord* CMSCollector::next_card_start_after_block(HeapWord* addr) const {
@@ -9212,7 +9210,6 @@
size_t MarkDeadObjectsClosure::do_blk(HeapWord* addr) {
size_t res = _sp->block_size_no_stall(addr, _collector);
- assert(res != 0, "Should always be able to compute a size");
if (_sp->block_is_obj(addr)) {
if (_live_bit_map->isMarked(addr)) {
// It can't have been dead in a previous cycle
@@ -9221,6 +9218,7 @@
_dead_bit_map->mark(addr); // mark the dead object
}
}
+ // Could be 0, if the block size could not be computed without stalling.
return res;
}
--- a/hotspot/src/share/vm/gc_interface/collectedHeap.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/gc_interface/collectedHeap.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -100,8 +100,7 @@
}
}
-void CollectedHeap::check_for_non_bad_heap_word_value(HeapWord* addr, size_t size)
- {
+void CollectedHeap::check_for_non_bad_heap_word_value(HeapWord* addr, size_t size) {
if (CheckMemoryInitialization && ZapUnusedHeapArea) {
for (size_t slot = 0; slot < size; slot += 1) {
assert((*(intptr_t*) (addr + slot)) == ((intptr_t) badHeapWordVal),
--- a/hotspot/src/share/vm/interpreter/rewriter.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/interpreter/rewriter.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -67,13 +67,11 @@
// Creates a constant pool cache given a CPC map
-// This creates the constant pool cache initially in a state
-// that is unsafe for concurrent GC processing but sets it to
-// a safe mode before the constant pool cache is returned.
void Rewriter::make_constant_pool_cache(TRAPS) {
const int length = _cp_cache_map.length();
constantPoolCacheOop cache =
- oopFactory::new_constantPoolCache(length, methodOopDesc::IsUnsafeConc, CHECK);
+ oopFactory::new_constantPoolCache(length, CHECK);
+ No_Safepoint_Verifier nsv;
cache->initialize(_cp_cache_map);
// Don't bother with the next pass if there is no JVM_CONSTANT_InvokeDynamic.
--- a/hotspot/src/share/vm/memory/oopFactory.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/memory/oopFactory.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -111,10 +111,9 @@
constantPoolCacheOop oopFactory::new_constantPoolCache(int length,
- bool is_conc_safe,
TRAPS) {
constantPoolCacheKlass* ck = constantPoolCacheKlass::cast(Universe::constantPoolCacheKlassObj());
- return ck->allocate(length, is_conc_safe, CHECK_NULL);
+ return ck->allocate(length, CHECK_NULL);
}
--- a/hotspot/src/share/vm/memory/oopFactory.hpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/memory/oopFactory.hpp Mon Feb 07 22:19:57 2011 -0800
@@ -69,7 +69,6 @@
bool is_conc_safe,
TRAPS);
static constantPoolCacheOop new_constantPoolCache(int length,
- bool is_conc_safe,
TRAPS);
// Instance classes
--- a/hotspot/src/share/vm/oops/constantPoolKlass.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/oops/constantPoolKlass.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -71,6 +71,12 @@
c->set_is_conc_safe(is_conc_safe);
// all fields are initialized; needed for GC
+ // Note: because we may be in this "conc_unsafe" state when allocating
+ // t_oop below, which may in turn cause a GC, it is imperative that our
+ // size be correct, consistent and henceforth stable, at this stage.
+ assert(c->is_parsable(), "Else size() below is unreliable");
+ DEBUG_ONLY(int sz = c->size();)
+
// initialize tag array
// Note: cannot introduce constant pool handle before since it is not
// completely initialized (no class) -> would cause assertion failure
@@ -82,6 +88,8 @@
}
pool->set_tags(tags());
+ // Check that our size was stable at its old value.
+ assert(sz == c->size(), "size() changed");
return pool();
}
--- a/hotspot/src/share/vm/oops/cpCacheKlass.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/oops/cpCacheKlass.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -49,43 +49,31 @@
constantPoolCacheOop constantPoolCacheKlass::allocate(int length,
- bool is_conc_safe,
TRAPS) {
// allocate memory
int size = constantPoolCacheOopDesc::object_size(length);
KlassHandle klass (THREAD, as_klassOop());
- // This is the original code. The code from permanent_obj_allocate()
- // was in-lined to allow the setting of is_conc_safe before the klass
- // is installed.
+ // Commented out below is the original code. The code from
+ // permanent_obj_allocate() was in-lined so that we could
+ // set the _length field, necessary to correctly compute its
+ // size(), before setting its klass word further below.
// constantPoolCacheOop cache = (constantPoolCacheOop)
// CollectedHeap::permanent_obj_allocate(klass, size, CHECK_NULL);
oop obj = CollectedHeap::permanent_obj_allocate_no_klass_install(klass, size, CHECK_NULL);
- constantPoolCacheOop cache = (constantPoolCacheOop) obj;
- cache->set_is_conc_safe(is_conc_safe);
- // The store to is_conc_safe must be visible before the klass
- // is set. This should be done safely because _is_conc_safe has
- // been declared volatile. If there are any problems, consider adding
- // OrderAccess::storestore();
- CollectedHeap::post_allocation_install_obj_klass(klass, obj, size);
NOT_PRODUCT(Universe::heap()->check_for_bad_heap_word_value((HeapWord*) obj,
size));
-
- // The length field affects the size of the object. The allocation
- // above allocates the correct size (see calculation of "size") but
- // the size() method of the constant pool cache oop will not reflect
- // that size until the correct length is set.
- cache->set_length(length);
+ constantPoolCacheOop cache = (constantPoolCacheOop) obj;
+ assert(!UseConcMarkSweepGC || obj->klass_or_null() == NULL,
+ "klass should be NULL here when using CMS");
+ cache->set_length(length); // should become visible before klass is set below.
+ cache->set_constant_pool(NULL);
- // The store of the length must be visible before is_conc_safe is
- // set to a safe state.
- // This should be done safely because _is_conc_safe has
- // been declared volatile. If there are any problems, consider adding
- // OrderAccess::storestore();
- cache->set_is_conc_safe(methodOopDesc::IsSafeConc);
- cache->set_constant_pool(NULL);
+ OrderAccess::storestore();
+ obj->set_klass(klass());
+ assert(cache->size() == size, "Incorrect cache->size()");
return cache;
}
@@ -176,11 +164,6 @@
return size;
}
-bool constantPoolCacheKlass::oop_is_conc_safe(oop obj) const {
- assert(obj->is_constantPoolCache(), "should be constant pool");
- return constantPoolCacheOop(obj)->is_conc_safe();
-}
-
#ifndef SERIALGC
void constantPoolCacheKlass::oop_push_contents(PSPromotionManager* pm,
oop obj) {
--- a/hotspot/src/share/vm/oops/cpCacheKlass.hpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/oops/cpCacheKlass.hpp Mon Feb 07 22:19:57 2011 -0800
@@ -39,7 +39,7 @@
// Allocation
DEFINE_ALLOCATE_PERMANENT(constantPoolCacheKlass);
- constantPoolCacheOop allocate(int length, bool is_conc_safe, TRAPS);
+ constantPoolCacheOop allocate(int length, TRAPS);
static klassOop create_klass(TRAPS);
// Casting from klassOop
@@ -55,7 +55,6 @@
// Garbage collection
void oop_follow_contents(oop obj);
int oop_adjust_pointers(oop obj);
- virtual bool oop_is_conc_safe(oop obj) const;
// Parallel Scavenge and Parallel Old
PARALLEL_GC_DECLS
--- a/hotspot/src/share/vm/oops/cpCacheOop.hpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/oops/cpCacheOop.hpp Mon Feb 07 22:19:57 2011 -0800
@@ -321,9 +321,6 @@
private:
int _length;
constantPoolOop _constant_pool; // the corresponding constant pool
- // If true, safe for concurrent GC processing,
- // Set unconditionally in constantPoolCacheKlass::allocate()
- volatile bool _is_conc_safe;
// Sizing
debug_only(friend class ClassVerifier;)
@@ -390,12 +387,6 @@
return entry_at(primary_index);
}
- // GC support
- // If the _length field has not been set, the size of the
- // constantPoolCache cannot be correctly calculated.
- bool is_conc_safe() { return _is_conc_safe; }
- void set_is_conc_safe(bool v) { _is_conc_safe = v; }
-
// Code generation
static ByteSize base_offset() { return in_ByteSize(sizeof(constantPoolCacheOopDesc)); }
static ByteSize entry_offset(int raw_index) {
--- a/hotspot/src/share/vm/oops/methodOop.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/oops/methodOop.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -985,9 +985,11 @@
IsUnsafeConc,
CHECK_(methodHandle()));
methodHandle newm (THREAD, newm_oop);
+ NOT_PRODUCT(int nmsz = newm->is_parsable() ? newm->size() : -1;)
int new_method_size = newm->method_size();
// Create a shallow copy of methodOopDesc part, but be careful to preserve the new constMethodOop
constMethodOop newcm = newm->constMethod();
+ NOT_PRODUCT(int ncmsz = newcm->is_parsable() ? newcm->size() : -1;)
int new_const_method_size = newm->constMethod()->object_size();
memcpy(newm(), m(), sizeof(methodOopDesc));
@@ -999,9 +1001,19 @@
// or concurrent marking but those phases will be correct. Setting and
// resetting is done in preference to a careful copying into newcm to
// avoid having to know the precise layout of a constMethodOop.
- m->constMethod()->set_is_conc_safe(false);
+ m->constMethod()->set_is_conc_safe(oopDesc::IsUnsafeConc);
+ assert(m->constMethod()->is_parsable(), "Should remain parsable");
+
+ // NOTE: this is a reachable object that transiently signals "conc_unsafe"
+ // However, no allocations are done during this window
+ // during which it is tagged conc_unsafe, so we are assured that any concurrent
+ // thread will not wait forever for the object to revert to "conc_safe".
+ // Further, any such conc_unsafe object will indicate a stable size
+ // through the transition.
memcpy(newcm, m->constMethod(), sizeof(constMethodOopDesc));
- m->constMethod()->set_is_conc_safe(true);
+ m->constMethod()->set_is_conc_safe(oopDesc::IsSafeConc);
+ assert(m->constMethod()->is_parsable(), "Should remain parsable");
+
// Reset correct method/const method, method size, and parameter info
newcm->set_method(newm());
newm->set_constMethod(newcm);
@@ -1035,6 +1047,8 @@
// Only set is_conc_safe to true when changes to newcm are
// complete.
+ assert(!newm->is_parsable() || nmsz < 0 || newm->size() == nmsz, "newm->size() inconsistency");
+ assert(!newcm->is_parsable() || ncmsz < 0 || newcm->size() == ncmsz, "newcm->size() inconsistency");
newcm->set_is_conc_safe(true);
return newm;
}
--- a/hotspot/src/share/vm/oops/methodOop.hpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/oops/methodOop.hpp Mon Feb 07 22:19:57 2011 -0800
@@ -144,9 +144,6 @@
public:
- static const bool IsUnsafeConc = false;
- static const bool IsSafeConc = true;
-
// accessors for instance variables
constMethodOop constMethod() const { return _constMethod; }
void set_constMethod(constMethodOop xconst) { oop_store_without_check((oop*)&_constMethod, (oop)xconst); }
--- a/hotspot/src/share/vm/oops/oop.hpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/oops/oop.hpp Mon Feb 07 22:19:57 2011 -0800
@@ -71,6 +71,11 @@
static BarrierSet* _bs;
public:
+ enum ConcSafeType {
+ IsUnsafeConc = false,
+ IsSafeConc = true
+ };
+
markOop mark() const { return _mark; }
markOop* mark_addr() const { return (markOop*) &_mark; }
--- a/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -1247,12 +1247,12 @@
// Constant pools are not easily reused so we allocate a new one
// each time.
// merge_cp is created unsafe for concurrent GC processing. It
- // should be marked safe before discarding it because, even if
- // garbage. If it crosses a card boundary, it may be scanned
+ // should be marked safe before discarding it. Even though
+ // garbage, if it crosses a card boundary, it may be scanned
// in order to find the start of the first complete object on the card.
constantPoolHandle merge_cp(THREAD,
oopFactory::new_constantPool(merge_cp_length,
- methodOopDesc::IsUnsafeConc,
+ oopDesc::IsUnsafeConc,
THREAD));
int orig_length = old_cp->orig_length();
if (orig_length == 0) {
@@ -2343,7 +2343,7 @@
// sized constant pool with the klass to save space.
constantPoolHandle smaller_cp(THREAD,
oopFactory::new_constantPool(scratch_cp_length,
- methodOopDesc::IsUnsafeConc,
+ oopDesc::IsUnsafeConc,
THREAD));
// preserve orig_length() value in the smaller copy
int orig_length = scratch_cp->orig_length();
--- a/hotspot/src/share/vm/prims/methodHandleWalk.cpp Thu Feb 03 20:49:09 2011 -0800
+++ b/hotspot/src/share/vm/prims/methodHandleWalk.cpp Mon Feb 07 22:19:57 2011 -0800
@@ -1134,8 +1134,9 @@
constantPoolHandle MethodHandleCompiler::get_constant_pool(TRAPS) const {
constantPoolHandle nullHandle;
- bool is_conc_safe = true;
- constantPoolOop cpool_oop = oopFactory::new_constantPool(_constants.length(), is_conc_safe, CHECK_(nullHandle));
+ constantPoolOop cpool_oop = oopFactory::new_constantPool(_constants.length(),
+ oopDesc::IsSafeConc,
+ CHECK_(nullHandle));
constantPoolHandle cpool(THREAD, cpool_oop);
// Fill the real constant pool skipping the zero element.
@@ -1180,10 +1181,9 @@
else
flags_bits = (/*JVM_MH_INVOKE_BITS |*/ JVM_ACC_PUBLIC | JVM_ACC_FINAL | JVM_ACC_SYNTHETIC);
- bool is_conc_safe = true;
methodOop m_oop = oopFactory::new_method(bytecode_length(),
accessFlags_from(flags_bits),
- 0, 0, 0, is_conc_safe, CHECK_(nullHandle));
+ 0, 0, 0, oopDesc::IsSafeConc, CHECK_(nullHandle));
methodHandle m(THREAD, m_oop);
m_oop = NULL; // oop not GC safe