Skip to content

Commit

Permalink
8322484: 22-b26 Regression in J2dBench-bimg_misc-G1 (and more) on Win…
Browse files Browse the repository at this point in the history
…dows-x64 and macOS-x64

Reviewed-by: kbarrett, ayang
  • Loading branch information
Thomas Schatzl committed Jan 29, 2024
1 parent af9cd97 commit 0d5f5e1
Show file tree
Hide file tree
Showing 13 changed files with 176 additions and 21 deletions.
5 changes: 5 additions & 0 deletions src/hotspot/share/gc/g1/g1BarrierSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "gc/g1/g1BarrierSetAssembler.hpp"
#include "gc/g1/g1CardTable.inline.hpp"
#include "gc/g1/g1CollectedHeap.inline.hpp"
#include "gc/g1/g1RegionPinCache.inline.hpp"
#include "gc/g1/g1SATBMarkQueueSet.hpp"
#include "gc/g1/g1ThreadLocalData.hpp"
#include "gc/g1/heapRegion.hpp"
Expand Down Expand Up @@ -171,4 +172,8 @@ void G1BarrierSet::on_thread_detach(Thread* thread) {
qset.flush_queue(queue);
qset.record_detached_refinement_stats(queue.refinement_stats());
}
{
G1RegionPinCache& cache = G1ThreadLocalData::pin_count_cache(thread);
cache.flush();
}
}
9 changes: 8 additions & 1 deletion src/hotspot/share/gc/g1/g1CollectedHeap.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -59,6 +59,7 @@
#include "gc/g1/g1PeriodicGCTask.hpp"
#include "gc/g1/g1Policy.hpp"
#include "gc/g1/g1RedirtyCardsQueue.hpp"
#include "gc/g1/g1RegionPinCache.inline.hpp"
#include "gc/g1/g1RegionToSpaceMapper.hpp"
#include "gc/g1/g1RemSet.hpp"
#include "gc/g1/g1RootClosures.hpp"
Expand Down Expand Up @@ -2471,6 +2472,12 @@ void G1CollectedHeap::retire_tlabs() {
ensure_parsability(true);
}

void G1CollectedHeap::flush_region_pin_cache() {
for (JavaThreadIteratorWithHandle jtiwh; JavaThread *thread = jtiwh.next(); ) {
G1ThreadLocalData::pin_count_cache(thread).flush();
}
}

void G1CollectedHeap::do_collection_pause_at_safepoint_helper() {
ResourceMark rm;

Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/share/gc/g1/g1CollectedHeap.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -775,6 +775,10 @@ class G1CollectedHeap : public CollectedHeap {

void retire_tlabs();

// Update all region's pin counts from the per-thread caches and resets them.
// Must be called before any decision based on pin counts.
void flush_region_pin_cache();

void expand_heap_after_young_collection();
// Update object copying statistics.
void record_obj_copy_mem_stats();
Expand Down
13 changes: 8 additions & 5 deletions src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -32,6 +32,7 @@
#include "gc/g1/g1ConcurrentMark.inline.hpp"
#include "gc/g1/g1EvacFailureRegions.hpp"
#include "gc/g1/g1Policy.hpp"
#include "gc/g1/g1RegionPinCache.inline.hpp"
#include "gc/g1/g1RemSet.hpp"
#include "gc/g1/heapRegion.inline.hpp"
#include "gc/g1/heapRegionManager.inline.hpp"
Expand Down Expand Up @@ -266,15 +267,17 @@ inline void G1CollectedHeap::pin_object(JavaThread* thread, oop obj) {
assert(obj != nullptr, "obj must not be null");
assert(!is_gc_active(), "must not pin objects during a GC");
assert(obj->is_typeArray(), "must be typeArray");
HeapRegion *r = heap_region_containing(obj);
r->increment_pinned_object_count();

uint obj_region_idx = heap_region_containing(obj)->hrm_index();
G1ThreadLocalData::pin_count_cache(thread).inc_count(obj_region_idx);
}

inline void G1CollectedHeap::unpin_object(JavaThread* thread, oop obj) {
assert(obj != nullptr, "obj must not be null");
assert(!is_gc_active(), "must not unpin objects during a GC");
HeapRegion *r = heap_region_containing(obj);
r->decrement_pinned_object_count();

uint obj_region_idx = heap_region_containing(obj)->hrm_index();
G1ThreadLocalData::pin_count_cache(thread).dec_count(obj_region_idx);
}

inline bool G1CollectedHeap::is_obj_dead(const oop obj) const {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/gc/g1/g1FullCollector.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -191,6 +191,7 @@ void G1FullCollector::prepare_collection() {

_heap->gc_prologue(true);
_heap->retire_tlabs();
_heap->flush_region_pin_cache();
_heap->prepare_heap_for_full_collection();

PrepareRegionsClosure cl(this);
Expand Down
57 changes: 57 additions & 0 deletions src/hotspot/share/gc/g1/g1RegionPinCache.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/

#ifndef SHARE_GC_G1_G1REGIONPINCACHE_HPP
#define SHARE_GC_G1_G1REGIONPINCACHE_HPP

#include "gc/g1/heapRegion.hpp"
#include "memory/allocation.hpp"
#include "utilities/globalDefinitions.hpp"

// Holds (caches) the pending pinned object count adjustment for the region
// _region_idx on a per thread basis.
// Keeping such a cache avoids the expensive atomic operations when updating the
// pin count for the very common case that the application pins and unpins the
// same object without any interleaving by a garbage collection or pinning/unpinning
// to an object in another region.
class G1RegionPinCache : public StackObj {
uint _region_idx;
size_t _count;

void flush_and_set(uint new_region_idx, size_t new_count);

public:
G1RegionPinCache() : _region_idx(G1_NO_HRM_INDEX), _count(0) { }

#ifdef ASSERT
size_t count() const { return _count; }
#endif

void inc_count(uint region_idx);
void dec_count(uint region_idx);

void flush();
};

#endif /* SHARE_GC_G1_G1REGIONPINCACHE_HPP */
60 changes: 60 additions & 0 deletions src/hotspot/share/gc/g1/g1RegionPinCache.inline.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/

#ifndef SHARE_GC_G1_G1REGIONPINCACHE_INLINE_HPP
#define SHARE_GC_G1_G1REGIONPINCACHE_INLINE_HPP

#include "gc/g1/g1RegionPinCache.hpp"

#include "gc/g1/g1CollectedHeap.inline.hpp"

inline void G1RegionPinCache::inc_count(uint region_idx) {
if (region_idx == _region_idx) {
++_count;
} else {
flush_and_set(region_idx, (size_t)1);
}
}

inline void G1RegionPinCache::dec_count(uint region_idx) {
if (region_idx == _region_idx) {
--_count;
} else {
flush_and_set(region_idx, ~(size_t)0);
}
}

inline void G1RegionPinCache::flush_and_set(uint new_region_idx, size_t new_count) {
if (_count != 0) {
G1CollectedHeap::heap()->region_at(_region_idx)->add_pinned_object_count(_count);
}
_region_idx = new_region_idx;
_count = new_count;
}

inline void G1RegionPinCache::flush() {
flush_and_set(G1_NO_HRM_INDEX, 0);
}

#endif /* SHARE_GC_G1_G1REGIONPINCACHE_INLINE_HPP */
15 changes: 13 additions & 2 deletions src/hotspot/share/gc/g1/g1ThreadLocalData.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -26,6 +26,7 @@

#include "gc/g1/g1BarrierSet.hpp"
#include "gc/g1/g1DirtyCardQueue.hpp"
#include "gc/g1/g1RegionPinCache.hpp"
#include "gc/shared/gc_globals.hpp"
#include "gc/shared/satbMarkQueue.hpp"
#include "runtime/javaThread.hpp"
Expand All @@ -37,9 +38,15 @@ class G1ThreadLocalData {
SATBMarkQueue _satb_mark_queue;
G1DirtyCardQueue _dirty_card_queue;

// Per-thread cache of pinned object count to reduce atomic operation traffic
// due to region pinning. Holds the last region where the mutator pinned an
// object and the number of pin operations since the last change of the region.
G1RegionPinCache _pin_cache;

G1ThreadLocalData() :
_satb_mark_queue(&G1BarrierSet::satb_mark_queue_set()),
_dirty_card_queue(&G1BarrierSet::dirty_card_queue_set()) {}
_dirty_card_queue(&G1BarrierSet::dirty_card_queue_set()),
_pin_cache() {}

static G1ThreadLocalData* data(Thread* thread) {
assert(UseG1GC, "Sanity");
Expand Down Expand Up @@ -90,6 +97,10 @@ class G1ThreadLocalData {
static ByteSize dirty_card_queue_buffer_offset() {
return dirty_card_queue_offset() + G1DirtyCardQueue::byte_offset_of_buf();
}

static G1RegionPinCache& pin_count_cache(Thread* thread) {
return data(thread)->_pin_cache;
}
};

#endif // SHARE_GC_G1_G1THREADLOCALDATA_HPP
1 change: 1 addition & 0 deletions src/hotspot/share/gc/g1/g1YoungCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "gc/g1/g1ParScanThreadState.inline.hpp"
#include "gc/g1/g1Policy.hpp"
#include "gc/g1/g1RedirtyCardsQueue.hpp"
#include "gc/g1/g1RegionPinCache.inline.hpp"
#include "gc/g1/g1RemSet.hpp"
#include "gc/g1/g1RootProcessor.hpp"
#include "gc/g1/g1Trace.hpp"
Expand Down
11 changes: 9 additions & 2 deletions src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -28,6 +28,8 @@
#include "gc/g1/g1ConcurrentRefineStats.hpp"
#include "gc/g1/g1DirtyCardQueue.hpp"
#include "gc/g1/g1YoungGCPreEvacuateTasks.hpp"
#include "gc/g1/g1RegionPinCache.inline.hpp"
#include "gc/g1/g1ThreadLocalData.hpp"
#include "gc/shared/barrierSet.inline.hpp"
#include "gc/shared/threadLocalAllocBuffer.inline.hpp"
#include "memory/allocation.inline.hpp"
Expand Down Expand Up @@ -57,12 +59,15 @@ class G1PreEvacuateCollectionSetBatchTask::JavaThreadRetireTLABAndFlushLogs : pu
assert(thread->is_Java_thread(), "must be");
// Flushes deferred card marks, so must precede concatenating logs.
BarrierSet::barrier_set()->make_parsable((JavaThread*)thread);
// Retire TLABs.
if (UseTLAB) {
thread->tlab().retire(&_tlab_stats);
}

// Concatenate logs.
G1DirtyCardQueueSet& qset = G1BarrierSet::dirty_card_queue_set();
_refinement_stats += qset.concatenate_log_and_stats(thread);
// Flush region pin count cache.
G1ThreadLocalData::pin_count_cache(thread).flush();
}
};

Expand Down Expand Up @@ -132,6 +137,8 @@ class G1PreEvacuateCollectionSetBatchTask::NonJavaThreadFlushLogs : public G1Abs
void do_thread(Thread* thread) override {
G1DirtyCardQueueSet& qset = G1BarrierSet::dirty_card_queue_set();
_refinement_stats += qset.concatenate_log_and_stats(thread);

assert(G1ThreadLocalData::pin_count_cache(thread).count() == 0, "NonJava thread has pinned Java objects");
}
} _tc;

Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/share/gc/g1/heapRegion.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,9 @@ class HeapRegion : public CHeapObj<mtGC> {
static uint LogOfHRGrainBytes;
static uint LogCardsPerRegion;

inline void increment_pinned_object_count();
inline void decrement_pinned_object_count();
// Atomically adjust the pinned object count by the given value. Value must not
// be zero.
inline void add_pinned_object_count(size_t value);

static size_t GrainBytes;
static size_t GrainWords;
Expand Down
10 changes: 4 additions & 6 deletions src/hotspot/share/gc/g1/heapRegion.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -553,12 +553,10 @@ inline void HeapRegion::record_surv_words_in_group(size_t words_survived) {
_surv_rate_group->record_surviving_words(age, words_survived);
}

inline void HeapRegion::increment_pinned_object_count() {
Atomic::add(&_pinned_object_count, (size_t)1, memory_order_relaxed);
}

inline void HeapRegion::decrement_pinned_object_count() {
Atomic::sub(&_pinned_object_count, (size_t)1, memory_order_relaxed);
inline void HeapRegion::add_pinned_object_count(size_t value) {
assert(value != 0, "wasted effort");
assert(!is_free(), "trying to pin free region %u, adding %zu", hrm_index(), value);
Atomic::add(&_pinned_object_count, value, memory_order_relaxed);
}

#endif // SHARE_GC_G1_HEAPREGION_INLINE_HPP

1 comment on commit 0d5f5e1

@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.