# HG changeset patch # User ysr # Date 1297145997 28800 # Node ID b1c2163e4e592deab81e73f6864f5fc21b2783b6 # Parent a2b2a8a3aaeed32db2b85203ee30193fbaa45db5 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 diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/classfile/classFileParser.cpp --- 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); diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp --- 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); } } diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp --- 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; } diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/gc_interface/collectedHeap.cpp --- 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), diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/interpreter/rewriter.cpp --- 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. diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/memory/oopFactory.cpp --- 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); } diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/memory/oopFactory.hpp --- 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 diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/oops/constantPoolKlass.cpp --- 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(); } diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/oops/cpCacheKlass.cpp --- 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) { diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/oops/cpCacheKlass.hpp --- 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 diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/oops/cpCacheOop.hpp --- 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) { diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/oops/methodOop.cpp --- 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; } diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/oops/methodOop.hpp --- 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); } diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/oops/oop.hpp --- 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; } diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp --- 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(); diff -r a2b2a8a3aaee -r b1c2163e4e59 hotspot/src/share/vm/prims/methodHandleWalk.cpp --- 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