diff --git a/src/hotspot/share/interpreter/oopMapCache.cpp b/src/hotspot/share/interpreter/oopMapCache.cpp index a10f8c439eaa4..87b124e9d7968 100644 --- a/src/hotspot/share/interpreter/oopMapCache.cpp +++ b/src/hotspot/share/interpreter/oopMapCache.cpp @@ -66,9 +66,6 @@ class OopMapCacheEntry: private InterpreterOopMap { public: OopMapCacheEntry() : InterpreterOopMap() { _next = nullptr; -#ifdef ASSERT - _resource_allocate_bit_mask = false; -#endif } }; @@ -177,9 +174,13 @@ class VerifyClosure : public OffsetClosure { InterpreterOopMap::InterpreterOopMap() { initialize(); -#ifdef ASSERT - _resource_allocate_bit_mask = true; -#endif +} + +InterpreterOopMap::~InterpreterOopMap() { + if (has_valid_mask() && mask_size() > small_mask_limit) { + assert(_bit_mask[0] != 0, "should have pointer to C heap"); + FREE_C_HEAP_ARRAY(uintptr_t, _bit_mask[0]); + } } bool InterpreterOopMap::is_empty() const { @@ -399,37 +400,24 @@ void OopMapCacheEntry::deallocate(OopMapCacheEntry* const entry) { // Implementation of OopMapCache -void InterpreterOopMap::resource_copy(OopMapCacheEntry* from) { - assert(_resource_allocate_bit_mask, - "Should not resource allocate the _bit_mask"); - assert(from->has_valid_mask(), - "Cannot copy entry with an invalid mask"); +void InterpreterOopMap::copy_from(const OopMapCacheEntry* src) { + // The expectation is that this InterpreterOopMap is recently created + // and empty. It is used to get a copy of a cached entry. + assert(!has_valid_mask(), "InterpreterOopMap object can only be filled once"); + assert(src->has_valid_mask(), "Cannot copy entry with an invalid mask"); - set_method(from->method()); - set_bci(from->bci()); - set_mask_size(from->mask_size()); - set_expression_stack_size(from->expression_stack_size()); - _num_oops = from->num_oops(); + set_method(src->method()); + set_bci(src->bci()); + set_mask_size(src->mask_size()); + set_expression_stack_size(src->expression_stack_size()); + _num_oops = src->num_oops(); // Is the bit mask contained in the entry? - if (from->mask_size() <= small_mask_limit) { - memcpy((void *)_bit_mask, (void *)from->_bit_mask, - mask_word_size() * BytesPerWord); + if (src->mask_size() <= small_mask_limit) { + memcpy(_bit_mask, src->_bit_mask, mask_word_size() * BytesPerWord); } else { - // The expectation is that this InterpreterOopMap is a recently created - // and empty. It is used to get a copy of a cached entry. - // If the bit mask has a value, it should be in the - // resource area. - assert(_bit_mask[0] == 0 || - Thread::current()->resource_area()->contains((void*)_bit_mask[0]), - "The bit mask should have been allocated from a resource area"); - // Allocate the bit_mask from a Resource area for performance. Allocating - // from the C heap as is done for OopMapCache has a significant - // performance impact. - _bit_mask[0] = (uintptr_t) NEW_RESOURCE_ARRAY(uintptr_t, mask_word_size()); - assert(_bit_mask[0] != 0, "bit mask was not allocated"); - memcpy((void*) _bit_mask[0], (void*) from->_bit_mask[0], - mask_word_size() * BytesPerWord); + _bit_mask[0] = (uintptr_t) NEW_C_HEAP_ARRAY(uintptr_t, mask_word_size(), mtClass); + memcpy((void*) _bit_mask[0], (void*) src->_bit_mask[0], mask_word_size() * BytesPerWord); } } @@ -512,7 +500,7 @@ void OopMapCache::lookup(const methodHandle& method, for (int i = 0; i < probe_depth; i++) { OopMapCacheEntry *entry = entry_at(probe + i); if (entry != nullptr && !entry->is_empty() && entry->match(method, bci)) { - entry_for->resource_copy(entry); + entry_for->copy_from(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; @@ -526,7 +514,7 @@ void OopMapCache::lookup(const methodHandle& method, OopMapCacheEntry* tmp = NEW_C_HEAP_OBJ(OopMapCacheEntry, mtClass); tmp->initialize(); tmp->fill(method, bci); - entry_for->resource_copy(tmp); + entry_for->copy_from(tmp); if (method->should_not_be_cached()) { // It is either not safe or not a good idea to cache this Method* @@ -627,7 +615,7 @@ void OopMapCache::compute_one_oop_map(const methodHandle& method, int bci, Inter tmp->initialize(); tmp->fill(method, bci); if (tmp->has_valid_mask()) { - entry->resource_copy(tmp); + entry->copy_from(tmp); } OopMapCacheEntry::deallocate(tmp); } diff --git a/src/hotspot/share/interpreter/oopMapCache.hpp b/src/hotspot/share/interpreter/oopMapCache.hpp index 7037b2c7d1fcf..062b4178ce06e 100644 --- a/src/hotspot/share/interpreter/oopMapCache.hpp +++ b/src/hotspot/share/interpreter/oopMapCache.hpp @@ -36,13 +36,14 @@ // OopMapCache's are allocated lazily per InstanceKlass. // The oopMap (InterpreterOopMap) is stored as a bit mask. If the -// bit_mask can fit into two words it is stored in +// bit_mask can fit into four words it is stored in // the _bit_mask array, otherwise it is allocated on the heap. // For OopMapCacheEntry the bit_mask is allocated in the C heap // because these entries persist between garbage collections. -// For InterpreterOopMap the bit_mask is allocated in -// a resource area for better performance. InterpreterOopMap -// should only be created and deleted during same garbage collection. +// For InterpreterOopMap the bit_mask is allocated in the C heap +// to avoid issues with allocations from the resource area that have +// to live accross the oop closure. InterpreterOopMap should only be +// created and deleted during the same garbage collection. // // If ENABBLE_ZAP_DEAD_LOCALS is defined, two bits are used // per entry instead of one. In all cases, @@ -88,9 +89,6 @@ class InterpreterOopMap: ResourceObj { unsigned short _bci; // the bci for which the mask is valid protected: -#ifdef ASSERT - bool _resource_allocate_bit_mask; -#endif int _num_oops; intptr_t _bit_mask[N]; // the bit mask if // mask_size <= small_mask_limit, @@ -128,12 +126,13 @@ class InterpreterOopMap: ResourceObj { public: InterpreterOopMap(); + ~InterpreterOopMap(); - // Copy the OopMapCacheEntry in parameter "from" into this - // InterpreterOopMap. If the _bit_mask[0] in "from" points to - // allocated space (i.e., the bit mask was to large to hold - // in-line), allocate the space from a Resource area. - void resource_copy(OopMapCacheEntry* from); + // Copy the OopMapCacheEntry in parameter "src" into this + // InterpreterOopMap. If the _bit_mask[0] in "src" points to + // allocated space (i.e., the bit mask was too large to hold + // in-line), allocate the space from the C heap. + void copy_from(const OopMapCacheEntry* src); void iterate_oop(OffsetClosure* oop_closure) const; void print() const; diff --git a/src/hotspot/share/runtime/frame.cpp b/src/hotspot/share/runtime/frame.cpp index 8f5d2ad4acbf1..1aed46d58804f 100644 --- a/src/hotspot/share/runtime/frame.cpp +++ b/src/hotspot/share/runtime/frame.cpp @@ -947,7 +947,6 @@ void frame::oops_interpreted_do(OopClosure* f, const RegisterMap* map, bool quer InterpreterFrameClosure blk(this, max_locals, m->max_stack(), f); // process locals & expression stack - ResourceMark rm(thread); InterpreterOopMap mask; if (query_oop_map_cache) { m->mask_for(m, bci, &mask);