--- old/src/share/vm/gc/shared/vmGCOperations.cpp 2017-08-11 11:30:11.067642987 -0400 +++ new/src/share/vm/gc/shared/vmGCOperations.cpp 2017-08-11 11:30:10.661236319 -0400 @@ -30,6 +30,7 @@ #include "gc/shared/gcLocker.inline.hpp" #include "gc/shared/genCollectedHeap.hpp" #include "gc/shared/vmGCOperations.hpp" +#include "interpreter/oopMapCache.hpp" #include "logging/log.hpp" #include "memory/oopFactory.hpp" #include "runtime/handles.inline.hpp" @@ -111,6 +112,9 @@ void VM_GC_Operation::doit_epilogue() { assert(Thread::current()->is_Java_thread(), "just checking"); + // Clean up old interpreter OopMap entries that were replaced + // during the GC thread root traversal. + OopMapCache::cleanup_old_entries(); if (Universe::has_reference_pending_list()) { Heap_lock->notify_all(); } --- old/src/share/vm/interpreter/oopMapCache.cpp 2017-08-11 11:30:19.082245508 -0400 +++ new/src/share/vm/interpreter/oopMapCache.cpp 2017-08-11 11:30:18.726328813 -0400 @@ -25,6 +25,7 @@ #include "precompiled.hpp" #include "interpreter/oopMapCache.hpp" #include "logging/log.hpp" +#include "logging/logStream.hpp" #include "memory/allocation.inline.hpp" #include "memory/resourceArea.hpp" #include "oops/oop.inline.hpp" @@ -37,6 +38,9 @@ friend class OopMapCache; friend class VerifyClosure; + private: + OopMapCacheEntry* _next; + protected: // Initialization void fill(const methodHandle& method, int bci); @@ -54,6 +58,7 @@ public: OopMapCacheEntry() : InterpreterOopMap() { + _next = NULL; #ifdef ASSERT _resource_allocate_bit_mask = false; #endif @@ -263,23 +268,26 @@ // Check if map is generated correctly // (Use ?: operator to make sure all 'true' & 'false' are represented exactly the same so we can use == afterwards) - if (TraceOopMapGeneration && Verbose) tty->print("Locals (%d): ", max_locals); + Log(interpreter, oopmap) logv; + LogStream st(logv.trace()); + st.print("Locals (%d): ", max_locals); for(int i = 0; i < max_locals; i++) { bool v1 = is_oop(i) ? true : false; bool v2 = vars[i].is_reference() ? true : false; assert(v1 == v2, "locals oop mask generation error"); - if (TraceOopMapGeneration && Verbose) tty->print("%d", v1 ? 1 : 0); + st.print("%d", v1 ? 1 : 0); } + st.cr(); - if (TraceOopMapGeneration && Verbose) { tty->cr(); tty->print("Stack (%d): ", stack_top); } + st.print("Stack (%d): ", stack_top); for(int j = 0; j < stack_top; j++) { bool v1 = is_oop(max_locals + j) ? true : false; bool v2 = stack[j].is_reference() ? true : false; assert(v1 == v2, "stack oop mask generation error"); - if (TraceOopMapGeneration && Verbose) tty->print("%d", v1 ? 1 : 0); + st.print("%d", v1 ? 1 : 0); } - if (TraceOopMapGeneration && Verbose) tty->cr(); + st.cr(); return true; } @@ -373,8 +381,6 @@ // verify bit mask assert(verify_mask(vars, stack, max_locals, stack_top), "mask could not be verified"); - - } void OopMapCacheEntry::flush() { @@ -385,16 +391,6 @@ // Implementation of OopMapCache -#ifndef PRODUCT - -static long _total_memory_usage = 0; - -long OopMapCache::memory_usage() { - return _total_memory_usage; -} - -#endif - void InterpreterOopMap::resource_copy(OopMapCacheEntry* from) { assert(_resource_allocate_bit_mask, "Should not resource allocate the _bit_mask"); @@ -435,15 +431,11 @@ ^ ((unsigned int) method->size_of_parameters() << 6); } +OopMapCacheEntry* volatile OopMapCache::_old_entries = NULL; -OopMapCache::OopMapCache() : - _mut(Mutex::leaf, "An OopMapCache lock", true) -{ - _array = NEW_C_HEAP_ARRAY(OopMapCacheEntry, _size, mtClass); - // Cannot call flush for initialization, since flush - // will check if memory should be deallocated - for(int i = 0; i < _size; i++) _array[i].initialize(); - NOT_PRODUCT(_total_memory_usage += sizeof(OopMapCache) + (sizeof(OopMapCacheEntry) * _size);) +OopMapCache::OopMapCache() { + _array = NEW_C_HEAP_ARRAY(OopMapCacheEntry*, _size, mtClass); + for(int i = 0; i < _size; i++) _array[i] = NULL; } @@ -452,112 +444,150 @@ // Deallocate oop maps that are allocated out-of-line flush(); // Deallocate array - NOT_PRODUCT(_total_memory_usage -= sizeof(OopMapCache) + (sizeof(OopMapCacheEntry) * _size);) - FREE_C_HEAP_ARRAY(OopMapCacheEntry, _array); + FREE_C_HEAP_ARRAY(OopMapCacheEntry*, _array); } OopMapCacheEntry* OopMapCache::entry_at(int i) const { - return &_array[i % _size]; + return (OopMapCacheEntry*)OrderAccess::load_ptr_acquire(&(_array[i % _size])); +} + +bool OopMapCache::put_at(int i, OopMapCacheEntry* entry, OopMapCacheEntry* old) { + return Atomic::cmpxchg_ptr (entry, &_array[i % _size], old) == old; } void OopMapCache::flush() { - for (int i = 0; i < _size; i++) _array[i].flush(); + for (int i = 0; i < _size; i++) { + OopMapCacheEntry* entry = _array[i]; + if (entry != NULL) { + _array[i] = NULL; // no barrier, only called in OopMapCache destructor + entry->flush(); + FREE_C_HEAP_OBJ(entry); + } + } } void OopMapCache::flush_obsolete_entries() { - for (int i = 0; i < _size; i++) - if (!_array[i].is_empty() && _array[i].method()->is_old()) { + assert(SafepointSynchronize::is_at_safepoint(), "called by RedefineClasses in a safepoint"); + for (int i = 0; i < _size; i++) { + OopMapCacheEntry* entry = _array[i]; + if (entry != NULL && !entry->is_empty() && entry->method()->is_old()) { // Cache entry is occupied by an old redefined method and we don't want // to pin it down so flush the entry. if (log_is_enabled(Debug, redefine, class, oopmap)) { ResourceMark rm; - log_debug(redefine, class, oopmap) + log_debug(redefine, class, interpreter, oopmap) ("flush: %s(%s): cached entry @%d", - _array[i].method()->name()->as_C_string(), _array[i].method()->signature()->as_C_string(), i); + entry->method()->name()->as_C_string(), entry->method()->signature()->as_C_string(), i); } - _array[i].flush(); + _array[i] = NULL; + entry->flush(); + FREE_C_HEAP_OBJ(entry); } + } } void OopMapCache::lookup(const methodHandle& method, int bci, - InterpreterOopMap* entry_for) const { - MutexLocker x(&_mut); - - OopMapCacheEntry* entry = NULL; + InterpreterOopMap* entry_for) { + assert(SafepointSynchronize::is_at_safepoint(), "called by Thread root scan"); int probe = hash_value_for(method, bci); + int i; + OopMapCacheEntry* entry = NULL; + + if (log_is_enabled(Debug, interpreter, oopmap)) { + static int count = 0; + ResourceMark rm; + log_debug(interpreter, oopmap) + ("%d - Computing oopmap at bci %d for %s at hash %d", ++count, bci, + method()->name_and_sig_as_C_string(), probe); + } // Search hashtable for match - int i; for(i = 0; i < _probe_depth; i++) { entry = entry_at(probe + i); - if (entry->match(method, bci)) { + if (entry != NULL && !entry->is_empty() && entry->match(method, bci)) { entry_for->resource_copy(entry); assert(!entry_for->is_empty(), "A non-empty oop map should be returned"); + log_debug(interpreter, oopmap)("- found at hash %d", probe + i); return; } } - if (TraceOopMapGeneration) { - static int count = 0; - ResourceMark rm; - tty->print("%d - Computing oopmap at bci %d for ", ++count, bci); - method->print_value(); tty->cr(); - } - // Entry is not in hashtable. - // Compute entry and return it + // Compute entry + + OopMapCacheEntry* tmp = NEW_C_HEAP_OBJ(OopMapCacheEntry, mtClass); + tmp->initialize(); + tmp->fill(method, bci); + entry_for->resource_copy(tmp); if (method->should_not_be_cached()) { // It is either not safe or not a good idea to cache this Method* // at this time. We give the caller of lookup() a copy of the // interesting info via parameter entry_for, but we don't add it to // the cache. See the gory details in Method*.cpp. - compute_one_oop_map(method, bci, entry_for); + FREE_C_HEAP_OBJ(tmp); return; } // First search for an empty slot for(i = 0; i < _probe_depth; i++) { - entry = entry_at(probe + i); - if (entry->is_empty()) { - entry->fill(method, bci); - entry_for->resource_copy(entry); - assert(!entry_for->is_empty(), "A non-empty oop map should be returned"); - return; + entry = entry_at(probe + i); + if (entry == NULL) { + if (put_at(probe + i, tmp, NULL)) { + assert(!entry_for->is_empty(), "A non-empty oop map should be returned"); + return; + } } } - if (TraceOopMapGeneration) { - ResourceMark rm; - tty->print_cr("*** collision in oopmap cache - flushing item ***"); - } + log_debug(interpreter, oopmap)("*** collision in oopmap cache - flushing item ***"); // No empty slot (uncommon case). Use (some approximation of a) LRU algorithm - //entry_at(probe + _probe_depth - 1)->flush(); - //for(i = _probe_depth - 1; i > 0; i--) { - // // Coping entry[i] = entry[i-1]; - // OopMapCacheEntry *to = entry_at(probe + i); - // OopMapCacheEntry *from = entry_at(probe + i - 1); - // to->copy(from); - // } - - assert(method->is_method(), "gaga"); + // where the first entry in the collision array is replaced with the new one. + OopMapCacheEntry* old = entry_at(probe + 0); + if (put_at(probe + 0, tmp, old)) { + enqueue_for_cleanup(old); + } else { + enqueue_for_cleanup(tmp); + } - entry = entry_at(probe + 0); - entry->fill(method, bci); + assert(!entry_for->is_empty(), "A non-empty oop map should be returned"); + return; +} - // Copy the newly cached entry to input parameter - entry_for->resource_copy(entry); +void OopMapCache::enqueue_for_cleanup(OopMapCacheEntry* entry) { + bool success = false; + OopMapCacheEntry* head; + do { + head = _old_entries; + entry->_next = head; + success = Atomic::cmpxchg_ptr (entry, &_old_entries, head) == head; + } while (!success); - if (TraceOopMapGeneration) { + if (log_is_enabled(Debug, interpreter, oopmap)) { ResourceMark rm; - tty->print("Done with "); - method->print_value(); tty->cr(); + log_debug(interpreter, oopmap)("enqueue %s at bci %d for cleanup", + entry->method()->name_and_sig_as_C_string(), entry->bci()); } - assert(!entry_for->is_empty(), "A non-empty oop map should be returned"); +} - return; +// This is called after GC threads are done and nothing is accessing the old_entries +// list, so no synchronization needed. +void OopMapCache::cleanup_old_entries() { + OopMapCacheEntry* entry = _old_entries; + _old_entries = NULL; + while (entry != NULL) { + if (log_is_enabled(Debug, interpreter, oopmap)) { + ResourceMark rm; + log_debug(interpreter, oopmap)("cleanup entry %s at bci %d", + entry->method()->name_and_sig_as_C_string(), entry->bci()); + } + OopMapCacheEntry* next = entry->_next; + entry->flush(); + FREE_C_HEAP_OBJ(entry); + entry = next; + } } void OopMapCache::compute_one_oop_map(const methodHandle& method, int bci, InterpreterOopMap* entry) { --- old/src/share/vm/interpreter/oopMapCache.hpp 2017-08-11 11:30:27.389394971 -0400 +++ new/src/share/vm/interpreter/oopMapCache.hpp 2017-08-11 11:30:27.123191451 -0400 @@ -144,17 +144,19 @@ }; class OopMapCache : public CHeapObj { + static OopMapCacheEntry* volatile _old_entries; private: enum { _size = 32, // Use fixed size for now _probe_depth = 3 // probe depth in case of collisions }; - OopMapCacheEntry* _array; + OopMapCacheEntry* volatile* _array; unsigned int hash_value_for(const methodHandle& method, int bci) const; OopMapCacheEntry* entry_at(int i) const; + bool put_at(int i, OopMapCacheEntry* entry, OopMapCacheEntry* old); - mutable Mutex _mut; + static void enqueue_for_cleanup(OopMapCacheEntry* entry); void flush(); @@ -167,13 +169,11 @@ // Returns the oopMap for (method, bci) in parameter "entry". // Returns false if an oop map was not found. - void lookup(const methodHandle& method, int bci, InterpreterOopMap* entry) const; + void lookup(const methodHandle& method, int bci, InterpreterOopMap* entry); // Compute an oop map without updating the cache or grabbing any locks (for debugging) static void compute_one_oop_map(const methodHandle& method, int bci, InterpreterOopMap* entry); - - // Returns total no. of bytes allocated as part of OopMapCache's - static long memory_usage() PRODUCT_RETURN0; + static void cleanup_old_entries(); }; #endif // SHARE_VM_INTERPRETER_OOPMAPCACHE_HPP --- old/src/share/vm/logging/logTag.hpp 2017-08-11 11:30:35.374092144 -0400 +++ new/src/share/vm/logging/logTag.hpp 2017-08-11 11:30:35.027330086 -0400 @@ -74,6 +74,7 @@ LOG_TAG(iklass) \ LOG_TAG(init) \ LOG_TAG(inlining) \ + LOG_TAG(interpreter) \ LOG_TAG(itables) \ LOG_TAG(jit) \ LOG_TAG(jni) \ --- old/src/share/vm/runtime/globals.hpp 2017-08-11 11:30:44.293222876 -0400 +++ new/src/share/vm/runtime/globals.hpp 2017-08-11 11:30:43.843035576 -0400 @@ -715,9 +715,6 @@ product(bool, PrintVMQWaitTime, false, \ "Print out the waiting time in VM operation queue") \ \ - develop(bool, TraceOopMapGeneration, false, \ - "Show OopMapGeneration") \ - \ product(bool, MethodFlushing, true, \ "Reclamation of zombie and not-entrant methods") \ \ --- old/src/share/vm/runtime/memprofiler.cpp 2017-08-11 11:30:52.391680041 -0400 +++ new/src/share/vm/runtime/memprofiler.cpp 2017-08-11 11:30:52.090763747 -0400 @@ -129,7 +129,7 @@ fprintf(_log_fp, UINTX_FORMAT_W(6) "," UINTX_FORMAT_W(6) ",%6ld\n", handles_memory_usage / K, resource_memory_usage / K, - OopMapCache::memory_usage() / K); + 0L); fflush(_log_fp); }