Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shared/oopStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ void OopStorage::Block::release_entries(uintx releasing, OopStorage* owner) {
// then someone else has made such a claim and the deferred update has not
// yet been processed and will include our change, so we don't need to do
// anything further.
if (_deferred_updates_next.compare_exchange(nullptr, this) == nullptr) {
if (_deferred_updates_next.compare_set(nullptr, this)) {
// Successfully claimed. Push, with self-loop for end-of-list.
Block* head = owner->_deferred_updates.load_relaxed();
while (true) {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shared/pretouchTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void PretouchTask::work(uint worker_id) {
char* cur_end = cur_start + MIN2(_chunk_size, pointer_delta(_end_addr, cur_start, 1));
if (cur_start >= cur_end) {
break;
} else if (cur_start == _cur_addr.compare_exchange(cur_start, cur_end)) {
} else if (_cur_addr.compare_set(cur_start, cur_end)) {
os::pretouch_memory(cur_start, cur_end, _page_size);
} // Else attempt to claim chunk failed, so try again.
}
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/gc/shared/taskqueue.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,8 @@ class TaskQueueSuper: public CHeapObj<MT> {
_age.store_relaxed(new_age);
}

Age cmpxchg_age(Age old_age, Age new_age) {
return _age.compare_exchange(old_age, new_age);
bool par_set_age(Age old_age, Age new_age) {
return _age.compare_set(old_age, new_age);
}

idx_t age_top_relaxed() const {
Expand Down Expand Up @@ -345,7 +345,7 @@ class GenericTaskQueue: public TaskQueueSuper<N, MT> {

using TaskQueueSuper<N, MT>::age_relaxed;
using TaskQueueSuper<N, MT>::set_age_relaxed;
using TaskQueueSuper<N, MT>::cmpxchg_age;
using TaskQueueSuper<N, MT>::par_set_age;
using TaskQueueSuper<N, MT>::age_top_relaxed;

using TaskQueueSuper<N, MT>::increment_index;
Expand Down
7 changes: 3 additions & 4 deletions src/hotspot/share/gc/shared/taskqueue.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ bool GenericTaskQueue<E, MT, N>::pop_local_slow(uint localBot, Age oldAge) {
if (localBot == oldAge.top()) {
// No competing pop_global has yet incremented "top"; we'll try to
// install new_age, thus claiming the element.
Age tempAge = cmpxchg_age(oldAge, newAge);
if (tempAge == oldAge) {
if (par_set_age(oldAge, newAge)) {
// We win.
assert_not_underflow(localBot, age_top_relaxed());
TASKQUEUE_STATS_ONLY(stats.record_pop_slow());
Expand Down Expand Up @@ -283,12 +282,12 @@ typename GenericTaskQueue<E, MT, N>::PopResult GenericTaskQueue<E, MT, N>::pop_g
idx_t new_top = increment_index(oldAge.top());
idx_t new_tag = oldAge.tag() + ((new_top == 0) ? 1 : 0);
Age newAge(new_top, new_tag);
Age resAge = cmpxchg_age(oldAge, newAge);
bool result = par_set_age(oldAge, newAge);

// Note that using "bottom" here might fail, since a pop_local might
// have decremented it.
assert_not_underflow(localBot, newAge.top());

Choose a reason for hiding this comment

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

[pre-existing] This assertion seems weird. It is checking newAge.top() after
already using been installed. Maybe it is misplaced? Or maybe it is checking
the wrong thing? Needs to be investigated. I've filed
https://bugs.openjdk.org/browse/JDK-8374915

return resAge == oldAge ? PopResult::Success : PopResult::Contended;
return result ? PopResult::Success : PopResult::Contended;
}

inline int randomParkAndMiller(int *seed0) {
Expand Down
13 changes: 13 additions & 0 deletions src/hotspot/share/runtime/atomic.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
// v.release_store(x) -> void
// v.release_store_fence(x) -> void
// v.compare_exchange(x, y [, o]) -> T
// v.compare_set(x, y [, o]) -> bool
// v.exchange(x [, o]) -> T
//
// (2) All atomic types are default constructible.
Expand Down Expand Up @@ -267,6 +268,11 @@ class AtomicImpl::CommonCore {
return AtomicAccess::cmpxchg(value_ptr(), compare_value, new_value, order);
}

bool compare_set(T compare_value, T new_value,
atomic_memory_order order = memory_order_conservative) {
return compare_exchange(compare_value, new_value, order) == compare_value;
}

T exchange(T new_value,
atomic_memory_order order = memory_order_conservative) {
return AtomicAccess::xchg(this->value_ptr(), new_value, order);
Expand Down Expand Up @@ -479,6 +485,13 @@ class AtomicImpl::Atomic<T, AtomicImpl::Category::Translated> {
order));
}

bool compare_set(T compare_value, T new_value,
atomic_memory_order order = memory_order_conservative) {
return _value.compare_set(decay(compare_value),
decay(new_value),
order);
Comment on lines +490 to +492
Copy link
Member

Choose a reason for hiding this comment

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

This is subtly different from what we usually did with AtomicAccess / Atomic compare_exchange on translated types. We used to check equality with the bool operator==(const T&, constT&) rather than bool operator==(const PrimitiveConversions::Translate<T>::Decayed&, const PrimitiveConversions::Translate<T>::Decayed&).

For the PrimitiveConversions::Translate we currently have I do not think there is an issues as they are memcmp equivalent in both cases. But we may introduce a PrimitiveConversion in the future where this is not the case and this function returns false when using recover and bool operator==(const T&, constT&) would have returned true.

}

T exchange(T new_value, atomic_memory_order order = memory_order_conservative) {
return recover(_value.exchange(decay(new_value), order));
}
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/utilities/concurrentHashTable.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ inline bool ConcurrentHashTable<CONFIG, MT>::
if (is_locked()) {
return false;
}
if (_first.compare_exchange(expect, node) == expect) {
if (_first.compare_set(expect, node)) {
return true;
}
return false;
Expand All @@ -172,7 +172,7 @@ inline bool ConcurrentHashTable<CONFIG, MT>::
}
// We will expect a clean first pointer.
Node* tmp = first();
if (_first.compare_exchange(tmp, set_state(tmp, STATE_LOCK_BIT)) == tmp) {
if (_first.compare_set(tmp, set_state(tmp, STATE_LOCK_BIT))) {
return true;
}
return false;
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/utilities/waitBarrier_generic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ void GenericWaitBarrier::Cell::disarm(int32_t expected_tag) {
tag, waiters);

int64_t new_state = encode(0, waiters);
if (_state.compare_exchange(state, new_state) == state) {
if (_state.compare_set(state, new_state)) {
// Successfully disarmed.
break;
}
Expand Down Expand Up @@ -218,7 +218,7 @@ void GenericWaitBarrier::Cell::wait(int32_t expected_tag) {
tag, waiters);

int64_t new_state = encode(tag, waiters + 1);
if (_state.compare_exchange(state, new_state) == state) {
if (_state.compare_set(state, new_state)) {
// Success! Proceed to wait.
break;
}
Expand Down Expand Up @@ -247,7 +247,7 @@ void GenericWaitBarrier::Cell::wait(int32_t expected_tag) {
tag, waiters);

int64_t new_state = encode(tag, waiters - 1);
if (_state.compare_exchange(state, new_state) == state) {
if (_state.compare_set(state, new_state)) {
// Success!
break;
}
Expand Down
29 changes: 29 additions & 0 deletions test/hotspot/gtest/runtime/test_atomic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,35 @@ TEST_VM(AtomicIntegerTest, cmpxchg_int64) {
Support().test();
}

template<typename T>
struct AtomicIntegerCmpsetTestSupport {
Atomic<T> _test_value;

AtomicIntegerCmpsetTestSupport() : _test_value{} {}

void test() {
T zero = 0;
T five = 5;
T ten = 10;
_test_value.store_relaxed(zero);
EXPECT_FALSE(_test_value.compare_set(five, ten));
EXPECT_EQ(zero, _test_value.load_relaxed());
EXPECT_TRUE(_test_value.compare_set(zero, ten));
EXPECT_EQ(ten, _test_value.load_relaxed());
}
};

TEST_VM(AtomicIntegerTest, cmpset_int32) {
using Support = AtomicIntegerCmpsetTestSupport<int32_t>;
Support().test();
}

TEST_VM(AtomicIntegerTest, cmpset_int64) {
// Check if 64-bit atomics are available on the machine.
using Support = AtomicIntegerCmpsetTestSupport<int64_t>;
Support().test();
}

struct AtomicXchgAndCmpxchg1ByteStressSupport {
char _default_val;
int _base;
Expand Down