Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Commit

Permalink
8322957: Generational ZGC: Relocation selection must join the STS
Browse files Browse the repository at this point in the history
Reviewed-by: aboldtch
Backport-of: ba23025cd8a9c1af37afea6444ce5ea2ff41e5af
  • Loading branch information
stefank committed Jan 16, 2024
1 parent 9257505 commit a91569d
Show file tree
Hide file tree
Showing 14 changed files with 148 additions and 41 deletions.
49 changes: 27 additions & 22 deletions src/hotspot/share/gc/shared/workerThread.cpp
Expand Up @@ -31,6 +31,7 @@
#include "runtime/init.hpp"
#include "runtime/java.hpp"
#include "runtime/os.hpp"
#include "runtime/safepoint.hpp"

WorkerTaskDispatcher::WorkerTaskDispatcher() :
_task(nullptr),
Expand Down Expand Up @@ -141,40 +142,44 @@ void WorkerThreads::threads_do(ThreadClosure* tc) const {
}
}

void WorkerThreads::set_indirectly_suspendible_threads() {
template <typename Function>
void WorkerThreads::threads_do_f(Function function) const {
for (uint i = 0; i < _created_workers; i++) {
function(_workers[i]);
}
}

void WorkerThreads::set_indirect_states() {
#ifdef ASSERT
class SetIndirectlySuspendibleThreadClosure : public ThreadClosure {
virtual void do_thread(Thread* thread) {
const bool is_suspendible = Thread::current()->is_suspendible_thread();
const bool is_safepointed = Thread::current()->is_VM_thread() && SafepointSynchronize::is_at_safepoint();

threads_do_f([&](Thread* thread) {
assert(!thread->is_indirectly_suspendible_thread(), "Unexpected");
assert(!thread->is_indirectly_safepoint_thread(), "Unexpected");
if (is_suspendible) {
thread->set_indirectly_suspendible_thread();
}
};

if (Thread::current()->is_suspendible_thread()) {
SetIndirectlySuspendibleThreadClosure cl;
threads_do(&cl);
}
if (is_safepointed) {
thread->set_indirectly_safepoint_thread();
}
});
#endif
}

void WorkerThreads::clear_indirectly_suspendible_threads() {
void WorkerThreads::clear_indirect_states() {
#ifdef ASSERT
class ClearIndirectlySuspendibleThreadClosure : public ThreadClosure {
virtual void do_thread(Thread* thread) {
thread->clear_indirectly_suspendible_thread();
}
};

if (Thread::current()->is_suspendible_thread()) {
ClearIndirectlySuspendibleThreadClosure cl;
threads_do(&cl);
}
threads_do_f([&](Thread* thread) {
thread->clear_indirectly_suspendible_thread();
thread->clear_indirectly_safepoint_thread();
});
#endif
}

void WorkerThreads::run_task(WorkerTask* task) {
set_indirectly_suspendible_threads();
set_indirect_states();
_dispatcher.coordinator_distribute_task(task, _active_workers);
clear_indirectly_suspendible_threads();
clear_indirect_states();
}

void WorkerThreads::run_task(WorkerTask* task, uint num_workers) {
Expand Down
6 changes: 4 additions & 2 deletions src/hotspot/share/gc/shared/workerThread.hpp
Expand Up @@ -93,8 +93,8 @@ class WorkerThreads : public CHeapObj<mtInternal> {

WorkerThread* create_worker(uint name_suffix);

void set_indirectly_suspendible_threads();
void clear_indirectly_suspendible_threads();
void set_indirect_states();
void clear_indirect_states();

protected:
virtual void on_create_worker(WorkerThread* worker) {}
Expand All @@ -111,6 +111,8 @@ class WorkerThreads : public CHeapObj<mtInternal> {
uint set_active_workers(uint num_workers);

void threads_do(ThreadClosure* tc) const;
template <typename Function>
void threads_do_f(Function function) const;

const char* name() const { return _name; }

Expand Down
13 changes: 2 additions & 11 deletions src/hotspot/share/gc/z/zBarrier.inline.hpp
Expand Up @@ -26,14 +26,13 @@

#include "gc/z/zBarrier.hpp"

#include "code/codeCache.hpp"
#include "gc/z/zAddress.inline.hpp"
#include "gc/z/zGeneration.inline.hpp"
#include "gc/z/zHeap.inline.hpp"
#include "gc/z/zResurrection.inline.hpp"
#include "gc/z/zVerify.hpp"
#include "oops/oop.hpp"
#include "runtime/atomic.hpp"
#include "runtime/continuation.hpp"

// A self heal must always "upgrade" the address metadata bits in
// accordance with the metadata bits state machine. The following
Expand Down Expand Up @@ -320,17 +319,9 @@ inline zaddress ZBarrier::make_load_good_no_relocate(zpointer o) {
return remap(ZPointer::uncolor_unsafe(o), remap_generation(o));
}

inline void z_assert_is_barrier_safe() {
assert(!Thread::current()->is_ConcurrentGC_thread() || /* Need extra checks for ConcurrentGCThreads */
Thread::current()->is_suspendible_thread() || /* Thread prevents safepoints */
Thread::current()->is_indirectly_suspendible_thread() || /* Coordinator thread prevents safepoints */
SafepointSynchronize::is_at_safepoint(), /* Is at safepoint */
"Shouldn't perform load barrier");
}

template <typename ZBarrierSlowPath>
inline zaddress ZBarrier::barrier(ZBarrierFastPath fast_path, ZBarrierSlowPath slow_path, ZBarrierColor color, volatile zpointer* p, zpointer o, bool allow_null) {
z_assert_is_barrier_safe();
z_verify_safepoints_are_blocked();

// Fast path
if (fast_path(o)) {
Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/share/gc/z/zGeneration.cpp
Expand Up @@ -286,6 +286,10 @@ void ZGeneration::desynchronize_relocation() {
_relocate.desynchronize();
}

bool ZGeneration::is_relocate_queue_active() const {
return _relocate.is_queue_active();
}

void ZGeneration::reset_statistics() {
assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
_freed = 0;
Expand Down Expand Up @@ -1496,7 +1500,7 @@ void ZGenerationOld::remap_young_roots() {
uint remap_nworkers = clamp(ZGeneration::young()->workers()->active_workers() + prev_nworkers, 1u, ZOldGCThreads);
_workers.set_active_workers(remap_nworkers);

// TODO: The STS joiner is only needed to satisfy z_assert_is_barrier_safe that doesn't
// TODO: The STS joiner is only needed to satisfy ZBarrier::assert_is_state_barrier_safe that doesn't
// understand the driver locker. Consider making the assert aware of the driver locker.
SuspendibleThreadSetJoiner sts_joiner;

Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/gc/z/zGeneration.hpp
Expand Up @@ -166,6 +166,7 @@ class ZGeneration {
// Relocation
void synchronize_relocation();
void desynchronize_relocation();
bool is_relocate_queue_active() const;
zaddress relocate_or_remap_object(zaddress_unsafe addr);
zaddress remap_object(zaddress_unsafe addr);

Expand Down
10 changes: 10 additions & 0 deletions src/hotspot/share/gc/z/zIterator.inline.hpp
Expand Up @@ -26,11 +26,21 @@

#include "gc/z/zIterator.hpp"

#include "gc/z/zVerify.hpp"
#include "memory/iterator.inline.hpp"
#include "oops/objArrayOop.hpp"
#include "oops/oop.inline.hpp"

inline bool ZIterator::is_invisible_object(oop obj) {
// This is a good place to make sure that we can't concurrently iterate over
// objects while VMThread operations think they have exclusive access to the
// object graph.
//
// One example that have caused problems is the JFR Leak Profiler, which
// sets the mark word to a value that makes the object arrays look like
// invisible objects.
z_verify_safepoints_are_blocked();

return obj->mark_acquire().is_marked();
}

Expand Down
26 changes: 23 additions & 3 deletions src/hotspot/share/gc/z/zRelocate.cpp
Expand Up @@ -87,6 +87,7 @@ ZRelocateQueue::ZRelocateQueue()
_nworkers(0),
_nsynchronized(0),
_synchronize(false),
_is_active(false),
_needs_attention(0) {}

bool ZRelocateQueue::needs_attention() const {
Expand All @@ -103,6 +104,20 @@ void ZRelocateQueue::dec_needs_attention() {
assert(needs_attention == 0 || needs_attention == 1, "Invalid state");
}

void ZRelocateQueue::activate(uint nworkers) {
_is_active = true;
join(nworkers);
}

void ZRelocateQueue::deactivate() {
Atomic::store(&_is_active, false);
clear();
}

bool ZRelocateQueue::is_active() const {
return Atomic::load(&_is_active);
}

void ZRelocateQueue::join(uint nworkers) {
assert(nworkers != 0, "Must request at least one worker");
assert(_nworkers == 0, "Invalid state");
Expand Down Expand Up @@ -327,7 +342,7 @@ ZWorkers* ZRelocate::workers() const {
}

void ZRelocate::start() {
_queue.join(workers()->active_workers());
_queue.activate(workers()->active_workers());
}

void ZRelocate::add_remset(volatile zpointer* p) {
Expand Down Expand Up @@ -1088,6 +1103,9 @@ class ZRelocateTask : public ZRestartableTask {

~ZRelocateTask() {
_generation->stat_relocation()->at_relocate_end(_small_allocator.in_place_count(), _medium_allocator.in_place_count());

// Signal that we're not using the queue anymore. Used mostly for asserts.
_queue->deactivate();
}

virtual void work() {
Expand Down Expand Up @@ -1232,8 +1250,6 @@ void ZRelocate::relocate(ZRelocationSet* relocation_set) {
ZRelocateAddRemsetForFlipPromoted task(relocation_set->flip_promoted_pages());
workers()->run(&task);
}

_queue.clear();
}

ZPageAge ZRelocate::compute_to_age(ZPageAge from_age) {
Expand Down Expand Up @@ -1316,3 +1332,7 @@ void ZRelocate::desynchronize() {
ZRelocateQueue* ZRelocate::queue() {
return &_queue;
}

bool ZRelocate::is_queue_active() const {
return _queue.is_active();
}
7 changes: 7 additions & 0 deletions src/hotspot/share/gc/z/zRelocate.hpp
Expand Up @@ -41,6 +41,7 @@ class ZRelocateQueue {
uint _nworkers;
uint _nsynchronized;
bool _synchronize;
volatile bool _is_active;
volatile int _needs_attention;

bool needs_attention() const;
Expand All @@ -53,6 +54,10 @@ class ZRelocateQueue {
public:
ZRelocateQueue();

void activate(uint nworkers);
void deactivate();
bool is_active() const;

void join(uint nworkers);
void resize_workers(uint nworkers);
void leave();
Expand Down Expand Up @@ -99,6 +104,8 @@ class ZRelocate {
void desynchronize();

ZRelocateQueue* queue();

bool is_queue_active() const;
};

#endif // SHARE_GC_Z_ZRELOCATE_HPP
7 changes: 7 additions & 0 deletions src/hotspot/share/gc/z/zRelocationSet.cpp
Expand Up @@ -106,18 +106,25 @@ class ZRelocationSetInstallTask : public ZTask {
}

virtual void work() {
// Join the STS to block out VMThreads while running promote_barrier_on_young_oop_field
SuspendibleThreadSetJoiner sts_joiner;

// Allocate and install forwardings for small pages
for (size_t page_index; _small_iter.next_index(&page_index);) {
ZPage* page = _small->at(int(page_index));
ZForwarding* const forwarding = ZForwarding::alloc(_allocator, page, to_age(page));
install_small(forwarding, _medium->length() + page_index);

SuspendibleThreadSet::yield();
}

// Allocate and install forwardings for medium pages
for (size_t page_index; _medium_iter.next_index(&page_index);) {
ZPage* page = _medium->at(int(page_index));
ZForwarding* const forwarding = ZForwarding::alloc(_allocator, page, to_age(page));
install_medium(forwarding, page_index);

SuspendibleThreadSet::yield();
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/gc/z/zUncoloredRoot.inline.hpp
Expand Up @@ -29,11 +29,12 @@
#include "gc/z/zAddress.inline.hpp"
#include "gc/z/zBarrier.inline.hpp"
#include "gc/z/zHeap.inline.hpp"
#include "gc/z/zBarrier.hpp"
#include "oops/oop.hpp"

template <typename ObjectFunctionT>
inline void ZUncoloredRoot::barrier(ObjectFunctionT function, zaddress_unsafe* p, uintptr_t color) {
z_assert_is_barrier_safe();
z_verify_safepoints_are_blocked();

const zaddress_unsafe addr = Atomic::load(p);
assert_is_valid(addr);
Expand Down
53 changes: 52 additions & 1 deletion src/hotspot/share/gc/z/zVerify.cpp
Expand Up @@ -43,16 +43,67 @@
#include "runtime/frame.inline.hpp"
#include "runtime/globals.hpp"
#include "runtime/handles.hpp"
#include "runtime/javaThread.hpp"
#include "runtime/javaThread.inline.hpp"
#include "runtime/mutexLocker.hpp"
#include "runtime/safepoint.hpp"
#include "runtime/stackFrameStream.inline.hpp"
#include "runtime/stackWatermark.inline.hpp"
#include "runtime/stackWatermarkSet.inline.hpp"
#include "runtime/thread.hpp"
#include "utilities/debug.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/preserveException.hpp"
#include "utilities/resourceHash.hpp"

#ifdef ASSERT

// Used to verify that safepoints operations can't be scheduled concurrently
// with callers to this function. Typically used to verify that object oops
// and headers are safe to access.
void z_verify_safepoints_are_blocked() {
Thread* current = Thread::current();

if (current->is_ConcurrentGC_thread()) {
assert(current->is_suspendible_thread(), // Thread prevents safepoints
"Safepoints are not blocked by current thread");

} else if (current->is_Worker_thread()) {
assert(// Check if ...
// the thread prevents safepoints
current->is_suspendible_thread() ||
// the coordinator thread is the safepointing VMThread
current->is_indirectly_safepoint_thread() ||
// the coordinator thread prevents safepoints
current->is_indirectly_suspendible_thread() ||
// the RelocateQueue prevents safepoints
//
// RelocateQueue acts as a pseudo STS leaver/joiner and blocks
// safepoints. There's currently no infrastructure to check if the
// current thread is active or not, so check the global states instead.
ZGeneration::young()->is_relocate_queue_active() ||
ZGeneration::old()->is_relocate_queue_active(),
"Safepoints are not blocked by current thread");

} else if (current->is_Java_thread()) {
JavaThreadState state = JavaThread::cast(current)->thread_state();
assert(state == _thread_in_Java || state == _thread_in_vm || state == _thread_new,
"Safepoints are not blocked by current thread from state: %d", state);

} else if (current->is_JfrSampler_thread()) {
// The JFR sampler thread blocks out safepoints with this lock.
assert_lock_strong(Threads_lock);

} else if (current->is_VM_thread()) {
// The VM Thread doesn't schedule new safepoints while executing
// other safepoint or handshake operations.

} else {
fatal("Unexpected thread type");
}
}

#endif

#define BAD_OOP_ARG(o, p) "Bad oop " PTR_FORMAT " found at " PTR_FORMAT, untype(o), p2i(p)

static bool z_is_null_relaxed(zpointer o) {
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/gc/z/zVerify.hpp
Expand Up @@ -30,6 +30,8 @@ class frame;
class ZForwarding;
class ZPageAllocator;

NOT_DEBUG(inline) void z_verify_safepoints_are_blocked() NOT_DEBUG_RETURN;

class ZVerify : public AllStatic {
private:
static void roots_strong(bool verify_after_old_mark);
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/thread.cpp
Expand Up @@ -73,6 +73,7 @@ Thread::Thread() {
set_lgrp_id(-1);
DEBUG_ONLY(clear_suspendible_thread();)
DEBUG_ONLY(clear_indirectly_suspendible_thread();)
DEBUG_ONLY(clear_indirectly_safepoint_thread();)

// allocated data structures
set_osthread(nullptr);
Expand Down

1 comment on commit a91569d

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