Skip to content

Commit

Permalink
Backport 7ab96c7
Browse files Browse the repository at this point in the history
  • Loading branch information
pchilano committed Jul 15, 2024
1 parent a106e52 commit ea7b13c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 49 deletions.
60 changes: 24 additions & 36 deletions src/hotspot/share/interpreter/oopMapCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ class OopMapCacheEntry: private InterpreterOopMap {
public:
OopMapCacheEntry() : InterpreterOopMap() {
_next = nullptr;
#ifdef ASSERT
_resource_allocate_bit_mask = false;
#endif
}
};

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -516,7 +504,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;
Expand All @@ -530,7 +518,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*
Expand Down Expand Up @@ -631,7 +619,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);
}
23 changes: 11 additions & 12 deletions src/hotspot/share/interpreter/oopMapCache.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -95,9 +96,6 @@ class InterpreterOopMap: ResourceObj {
// access it without using trickery in
// method bit_mask().
int _num_oops;
#ifdef ASSERT
bool _resource_allocate_bit_mask;
#endif

// access methods
Method* method() const { return _method; }
Expand Down Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/runtime/frame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit ea7b13c

Please sign in to comment.