Skip to content

Commit

Permalink
8237913: G1CollectedHeap::heap_region_containing shouldn't be a template
Browse files Browse the repository at this point in the history
Reviewed-by: kbarrett, sangheki
  • Loading branch information
Thomas Schatzl committed Jul 27, 2022
1 parent dc74ea2 commit 37b08c7
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1CollectedHeap.cpp
Expand Up @@ -2226,7 +2226,7 @@ void G1CollectedHeap::start_concurrent_gc_for_metadata_allocation(GCCause::Cause
}

bool G1CollectedHeap::is_in(const void* p) const {
return is_in_reserved(p) && _hrm.is_available(addr_to_region((HeapWord*)p));
return is_in_reserved(p) && _hrm.is_available(addr_to_region(p));
}

// Iteration functions.
Expand Down
14 changes: 6 additions & 8 deletions src/hotspot/share/gc/g1/g1CollectedHeap.hpp
Expand Up @@ -1077,7 +1077,7 @@ class G1CollectedHeap : public CollectedHeap {

// Calculate the region index of the given address. Given address must be
// within the heap.
inline uint addr_to_region(HeapWord* addr) const;
inline uint addr_to_region(const void* addr) const;

inline HeapWord* bottom_addr_for_region(uint index) const;

Expand Down Expand Up @@ -1121,14 +1121,12 @@ class G1CollectedHeap : public CollectedHeap {
size_t length,
uint worker_id) const;

// Returns the HeapRegion that contains addr. addr must not be NULL.
template <class T>
inline HeapRegion* heap_region_containing(const T addr) const;
// Returns the HeapRegion that contains addr. addr must not be nullptr.
inline HeapRegion* heap_region_containing(const void* addr) const;

// Returns the HeapRegion that contains addr, or NULL if that is an uncommitted
// region. addr must not be NULL.
template <class T>
inline HeapRegion* heap_region_containing_or_null(const T addr) const;
// Returns the HeapRegion that contains addr, or nullptr if that is an uncommitted
// region. addr must not be nullptr.
inline HeapRegion* heap_region_containing_or_null(const void* addr) const;

// A CollectedHeap is divided into a dense sequence of "blocks"; that is,
// each address in the (reserved) heap is a member of exactly
Expand Down
20 changes: 7 additions & 13 deletions src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp
Expand Up @@ -86,7 +86,7 @@ inline HeapRegion* G1CollectedHeap::next_region_in_humongous(HeapRegion* hr) con
return _hrm.next_region_in_humongous(hr);
}

inline uint G1CollectedHeap::addr_to_region(HeapWord* addr) const {
inline uint G1CollectedHeap::addr_to_region(const void* addr) const {
assert(is_in_reserved(addr),
"Cannot calculate region index for address " PTR_FORMAT " that is outside of the heap [" PTR_FORMAT ", " PTR_FORMAT ")",
p2i(addr), p2i(reserved().start()), p2i(reserved().end()));
Expand All @@ -97,21 +97,15 @@ inline HeapWord* G1CollectedHeap::bottom_addr_for_region(uint index) const {
return _hrm.reserved().start() + index * HeapRegion::GrainWords;
}

template <class T>
inline HeapRegion* G1CollectedHeap::heap_region_containing(const T addr) const {
assert(addr != NULL, "invariant");
assert(is_in_reserved((const void*) addr),
inline HeapRegion* G1CollectedHeap::heap_region_containing(const void* addr) const {
assert(addr != nullptr, "invariant");
assert(is_in_reserved(addr),
"Address " PTR_FORMAT " is outside of the heap ranging from [" PTR_FORMAT " to " PTR_FORMAT ")",
p2i((void*)addr), p2i(reserved().start()), p2i(reserved().end()));
return _hrm.addr_to_region((HeapWord*)(void*) addr);
return _hrm.addr_to_region((HeapWord*)addr);
}

template <class T>
inline HeapRegion* G1CollectedHeap::heap_region_containing_or_null(const T addr) const {
assert(addr != NULL, "invariant");
assert(is_in_reserved((const void*) addr),
"Address " PTR_FORMAT " is outside of the heap ranging from [" PTR_FORMAT " to " PTR_FORMAT ")",
p2i((void*)addr), p2i(reserved().start()), p2i(reserved().end()));
inline HeapRegion* G1CollectedHeap::heap_region_containing_or_null(const void* addr) const {
uint const region_idx = addr_to_region(addr);
return region_at_or_null(region_idx);
}
Expand Down Expand Up @@ -246,7 +240,7 @@ inline bool G1CollectedHeap::is_humongous_reclaim_candidate(uint region) {
}

inline void G1CollectedHeap::set_humongous_is_live(oop obj) {
uint region = addr_to_region(cast_from_oop<HeapWord*>(obj));
uint region = addr_to_region(obj);
// Reset the entry in the region attribute table so that subsequent
// references to the same humongous object do not go into the slow path
// again. This is racy, as multiple threads may at the same time enter here,
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp
Expand Up @@ -48,7 +48,7 @@ inline bool G1CMIsAliveClosure::do_object_b(oop obj) {
return true;
}

HeapRegion* hr = _g1h->heap_region_containing(cast_from_oop<HeapWord*>(obj));
HeapRegion* hr = _g1h->heap_region_containing(obj);
// All objects allocated since the start of marking are considered live.
if (hr->obj_allocated_since_marking_start(obj)) {
return true;
Expand Down Expand Up @@ -214,7 +214,7 @@ inline void G1ConcurrentMark::update_top_at_rebuild_start(HeapRegion* r) {
}

inline void G1CMTask::update_liveness(oop const obj, const size_t obj_size) {
_mark_stats_cache.add_live_words(_g1h->addr_to_region(cast_from_oop<HeapWord*>(obj)), obj_size);
_mark_stats_cache.add_live_words(_g1h->addr_to_region(obj), obj_size);
}

inline void G1ConcurrentMark::add_to_liveness(uint worker_id, oop const obj, size_t size) {
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/gc/g1/g1FullGCOopClosures.cpp
Expand Up @@ -73,14 +73,14 @@ template <class T> void G1VerifyOopClosure::do_oop_work(T* p) {
yy.print_cr("----------");
}
if (!_g1h->is_in(obj)) {
HeapRegion* from = _g1h->heap_region_containing((HeapWord*)p);
HeapRegion* from = _g1h->heap_region_containing(p);
yy.print_cr("Field " PTR_FORMAT " of live obj " PTR_FORMAT " in region " HR_FORMAT,
p2i(p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
print_object(&yy, _containing_obj);
yy.print_cr("points to obj " PTR_FORMAT " not in the heap",
p2i(obj));
} else {
HeapRegion* from = _g1h->heap_region_containing((HeapWord*)p);
HeapRegion* from = _g1h->heap_region_containing(p);
HeapRegion* to = _g1h->heap_region_containing(obj);
yy.print_cr("Field " PTR_FORMAT " of live obj " PTR_FORMAT " in region " HR_FORMAT,
p2i(p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1OopClosures.inline.hpp
Expand Up @@ -173,7 +173,7 @@ inline void G1ScanCardClosure::do_oop_work(T* p) {

assert(!_g1h->is_in_cset((HeapWord*)p),
"Oop originates from " PTR_FORMAT " (region: %u) which is in the collection set.",
p2i(p), _g1h->addr_to_region((HeapWord*)p));
p2i(p), _g1h->addr_to_region(p));

const G1HeapRegionAttr region_attr = _g1h->region_attr(obj);
if (region_attr.is_in_cset()) {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
Expand Up @@ -197,7 +197,7 @@ void G1ParScanThreadState::do_oop_evac(T* p) {
// as they are not added to the collection set due to above precondition.
assert(!region_attr.is_humongous(),
"Obj " PTR_FORMAT " should not refer to humongous region %u from " PTR_FORMAT,
p2i(obj), _g1h->addr_to_region(cast_from_oop<HeapWord*>(obj)), p2i(p));
p2i(obj), _g1h->addr_to_region(obj), p2i(p));

if (!region_attr.is_in_cset()) {
// In this case somebody else already did all the work.
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1RegionMarkStatsCache.cpp
Expand Up @@ -44,7 +44,7 @@ G1RegionMarkStatsCache::~G1RegionMarkStatsCache() {
}

void G1RegionMarkStatsCache::add_live_words(oop obj) {
uint region_index = G1CollectedHeap::heap()->addr_to_region(cast_from_oop<HeapWord*>(obj));
uint region_index = G1CollectedHeap::heap()->addr_to_region(obj);
add_live_words(region_index, obj->size());
}

Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/gc/g1/heapRegion.cpp
Expand Up @@ -529,7 +529,7 @@ class VerifyLiveClosure : public G1VerificationClosure {
}
ResourceMark rm;
if (!is_in_heap) {
HeapRegion* from = _g1h->heap_region_containing((HeapWord*)p);
HeapRegion* from = _g1h->heap_region_containing(p);
log.error("Field " PTR_FORMAT " of live obj " PTR_FORMAT " in region " HR_FORMAT,
p2i(p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
LogStream ls(log.error());
Expand All @@ -538,7 +538,7 @@ class VerifyLiveClosure : public G1VerificationClosure {
log.error("points to obj " PTR_FORMAT " in region " HR_FORMAT " remset %s",
p2i(obj), HR_FORMAT_PARAMS(to), to->rem_set()->get_state_str());
} else {
HeapRegion* from = _g1h->heap_region_containing((HeapWord*)p);
HeapRegion* from = _g1h->heap_region_containing(p);
HeapRegion* to = _g1h->heap_region_containing(obj);
log.error("Field " PTR_FORMAT " of live obj " PTR_FORMAT " in region " HR_FORMAT,
p2i(p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
Expand Down Expand Up @@ -577,7 +577,7 @@ class VerifyRemSetClosure : public G1VerificationClosure {
Log(gc, verify) log;
if (!CompressedOops::is_null(heap_oop)) {
oop obj = CompressedOops::decode_not_null(heap_oop);
HeapRegion* from = _g1h->heap_region_containing((HeapWord*)p);
HeapRegion* from = _g1h->heap_region_containing(p);
HeapRegion* to = _g1h->heap_region_containing(obj);
if (from != NULL && to != NULL &&
from != to &&
Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/share/gc/g1/heapRegionManager.inline.hpp
Expand Up @@ -40,9 +40,7 @@ inline HeapRegion* HeapRegionManager::addr_to_region(HeapWord* addr) const {
"addr: " PTR_FORMAT " end: " PTR_FORMAT, p2i(addr), p2i(heap_end()));
assert(addr >= heap_bottom(),
"addr: " PTR_FORMAT " bottom: " PTR_FORMAT, p2i(addr), p2i(heap_bottom()));

HeapRegion* hr = _regions.get_by_address(addr);
return hr;
return _regions.get_by_address(addr);
}

inline HeapRegion* HeapRegionManager::at(uint index) const {
Expand Down

1 comment on commit 37b08c7

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.