Skip to content

Commit 62d03a5

Browse files
committed
Use explicit memory orders in concurrent_set
The atomic load/store operations here should mostly be using release/acquire semantics. This may lead to better performance than what we had under the default seq_cst. On x86 this may make the atomic store of hash faster, as it can avoid xchg. On ARM the loads may be faster (depending on target CPU for the compiler). Reference for comparison of atomic operations https://godbolt.org/z/6EdaMa5rG
1 parent 2a3b19d commit 62d03a5

1 file changed

Lines changed: 18 additions & 19 deletions

File tree

concurrent_set.c

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#include "internal.h"
22
#include "internal/gc.h"
33
#include "internal/concurrent_set.h"
4-
#include "ruby_atomic.h"
54
#include "ruby/atomic.h"
65
#include "vm_sync.h"
76

@@ -134,7 +133,7 @@ static void
134133
concurrent_set_try_resize_without_locking(VALUE old_set_obj, VALUE *set_obj_ptr)
135134
{
136135
// Check if another thread has already resized.
137-
if (RUBY_ATOMIC_VALUE_LOAD(*set_obj_ptr) != old_set_obj) {
136+
if (rbimpl_atomic_value_load(set_obj_ptr, RBIMPL_ATOMIC_ACQUIRE) != old_set_obj) {
138137
return;
139138
}
140139

@@ -146,7 +145,7 @@ concurrent_set_try_resize_without_locking(VALUE old_set_obj, VALUE *set_obj_ptr)
146145

147146
// This may overcount by up to the number of threads concurrently attempting to insert
148147
// GC may also happen between now and the set being rebuilt
149-
int expected_size = RUBY_ATOMIC_LOAD(old_set->size) - old_set->deleted_entries;
148+
int expected_size = rbimpl_atomic_load(&old_set->size, RBIMPL_ATOMIC_RELAXED) - old_set->deleted_entries;
150149

151150
struct concurrent_set_entry *old_entries = old_set->entries;
152151
int old_capacity = old_set->capacity;
@@ -164,13 +163,13 @@ concurrent_set_try_resize_without_locking(VALUE old_set_obj, VALUE *set_obj_ptr)
164163

165164
for (int i = 0; i < old_capacity; i++) {
166165
struct concurrent_set_entry *entry = &old_entries[i];
167-
VALUE key = RUBY_ATOMIC_VALUE_EXCHANGE(entry->key, CONCURRENT_SET_MOVED);
166+
VALUE key = rbimpl_atomic_value_exchange(&entry->key, CONCURRENT_SET_MOVED, RBIMPL_ATOMIC_ACQUIRE);
168167
RUBY_ASSERT(key != CONCURRENT_SET_MOVED);
169168

170169
if (key < CONCURRENT_SET_SPECIAL_VALUE_COUNT) continue;
171170
if (!RB_SPECIAL_CONST_P(key) && rb_objspace_garbage_object_p(key)) continue;
172171

173-
VALUE hash = RUBY_ATOMIC_VALUE_LOAD(entry->hash);
172+
VALUE hash = rbimpl_atomic_value_load(&entry->hash, RBIMPL_ATOMIC_RELAXED);
174173
if (hash == 0) {
175174
// Either in-progress insert or extremely unlikely 0 hash.
176175
// Re-calculate the hash.
@@ -203,7 +202,7 @@ concurrent_set_try_resize_without_locking(VALUE old_set_obj, VALUE *set_obj_ptr)
203202
}
204203
}
205204

206-
RUBY_ATOMIC_VALUE_SET(*set_obj_ptr, new_set_obj);
205+
rbimpl_atomic_value_store(set_obj_ptr, new_set_obj, RBIMPL_ATOMIC_RELEASE);
207206

208207
RB_GC_GUARD(old_set_obj);
209208

@@ -230,7 +229,7 @@ rb_concurrent_set_find(VALUE *set_obj_ptr, VALUE key)
230229
VALUE hash = 0;
231230

232231
retry:
233-
set_obj = RUBY_ATOMIC_VALUE_LOAD(*set_obj_ptr);
232+
set_obj = rbimpl_atomic_value_load(set_obj_ptr, RBIMPL_ATOMIC_ACQUIRE);
234233
RUBY_ASSERT(set_obj);
235234
struct concurrent_set *set = RTYPEDDATA_GET_DATA(set_obj);
236235

@@ -246,7 +245,7 @@ rb_concurrent_set_find(VALUE *set_obj_ptr, VALUE key)
246245

247246
while (true) {
248247
struct concurrent_set_entry *entry = &set->entries[idx];
249-
VALUE curr_key = RUBY_ATOMIC_VALUE_LOAD(entry->key);
248+
VALUE curr_key = rbimpl_atomic_value_load(&entry->key, RBIMPL_ATOMIC_ACQUIRE);
250249

251250
switch (curr_key) {
252251
case CONCURRENT_SET_EMPTY:
@@ -259,13 +258,13 @@ rb_concurrent_set_find(VALUE *set_obj_ptr, VALUE key)
259258

260259
goto retry;
261260
default: {
262-
VALUE curr_hash = RUBY_ATOMIC_VALUE_LOAD(entry->hash);
261+
VALUE curr_hash = rbimpl_atomic_value_load(&entry->hash, RBIMPL_ATOMIC_RELAXED);
263262
if (curr_hash != 0 && curr_hash != hash) break;
264263

265264
if (UNLIKELY(!RB_SPECIAL_CONST_P(curr_key) && rb_objspace_garbage_object_p(curr_key))) {
266265
// This is a weakref set, so after marking but before sweeping is complete we may find a matching garbage object.
267266
// Skip it and mark it as deleted.
268-
RUBY_ATOMIC_VALUE_CAS(entry->key, curr_key, CONCURRENT_SET_DELETED);
267+
rbimpl_atomic_value_cas(&entry->key, curr_key, CONCURRENT_SET_DELETED, RBIMPL_ATOMIC_RELEASE, RBIMPL_ATOMIC_RELAXED);
269268
break;
270269
}
271270

@@ -293,7 +292,7 @@ rb_concurrent_set_find_or_insert(VALUE *set_obj_ptr, VALUE key, void *data)
293292
VALUE hash = 0;
294293

295294
retry:
296-
set_obj = RUBY_ATOMIC_VALUE_LOAD(*set_obj_ptr);
295+
set_obj = rbimpl_atomic_value_load(set_obj_ptr, RBIMPL_ATOMIC_ACQUIRE);
297296
RUBY_ASSERT(set_obj);
298297
struct concurrent_set *set = RTYPEDDATA_GET_DATA(set_obj);
299298

@@ -309,7 +308,7 @@ rb_concurrent_set_find_or_insert(VALUE *set_obj_ptr, VALUE key, void *data)
309308

310309
while (true) {
311310
struct concurrent_set_entry *entry = &set->entries[idx];
312-
VALUE curr_key = RUBY_ATOMIC_VALUE_LOAD(entry->key);
311+
VALUE curr_key = rbimpl_atomic_value_load(&entry->key, RBIMPL_ATOMIC_ACQUIRE);
313312

314313
switch (curr_key) {
315314
case CONCURRENT_SET_EMPTY: {
@@ -320,7 +319,7 @@ rb_concurrent_set_find_or_insert(VALUE *set_obj_ptr, VALUE key, void *data)
320319
inserting = true;
321320
}
322321

323-
rb_atomic_t prev_size = RUBY_ATOMIC_FETCH_ADD(set->size, 1);
322+
rb_atomic_t prev_size = rbimpl_atomic_fetch_add(&set->size, 1, RBIMPL_ATOMIC_RELAXED);
324323

325324
double load_factor = (1.0 * prev_size) / (set->capacity);
326325

@@ -330,16 +329,16 @@ rb_concurrent_set_find_or_insert(VALUE *set_obj_ptr, VALUE key, void *data)
330329
goto retry;
331330
}
332331

333-
curr_key = RUBY_ATOMIC_VALUE_CAS(entry->key, CONCURRENT_SET_EMPTY, key);
332+
curr_key = rbimpl_atomic_value_cas(&entry->key, CONCURRENT_SET_EMPTY, key, RBIMPL_ATOMIC_RELEASE, RBIMPL_ATOMIC_RELAXED);
334333
if (curr_key == CONCURRENT_SET_EMPTY) {
335-
RUBY_ATOMIC_VALUE_SET(entry->hash, hash);
334+
rbimpl_atomic_value_store(&entry->hash, hash, RBIMPL_ATOMIC_RELAXED);
336335

337336
RB_GC_GUARD(set_obj);
338337
return key;
339338
}
340339
else {
341340
// Entry was not inserted.
342-
RUBY_ATOMIC_DEC(set->size);
341+
rbimpl_atomic_sub(&set->size, 1, RBIMPL_ATOMIC_RELAXED);
343342

344343
// Another thread won the race, try again at the same location.
345344
continue;
@@ -353,13 +352,13 @@ rb_concurrent_set_find_or_insert(VALUE *set_obj_ptr, VALUE key, void *data)
353352

354353
goto retry;
355354
default: {
356-
VALUE curr_hash = RUBY_ATOMIC_VALUE_LOAD(entry->hash);
355+
VALUE curr_hash = rbimpl_atomic_value_load(&entry->hash, RBIMPL_ATOMIC_RELAXED);
357356
if (curr_hash != 0 && curr_hash != hash) break;
358357

359358
if (UNLIKELY(!RB_SPECIAL_CONST_P(curr_key) && rb_objspace_garbage_object_p(curr_key))) {
360359
// This is a weakref set, so after marking but before sweeping is complete we may find a matching garbage object.
361360
// Skip it and mark it as deleted.
362-
RUBY_ATOMIC_VALUE_CAS(entry->key, curr_key, CONCURRENT_SET_DELETED);
361+
rbimpl_atomic_value_cas(&entry->key, curr_key, CONCURRENT_SET_DELETED, RBIMPL_ATOMIC_RELEASE, RBIMPL_ATOMIC_RELAXED);
363362
break;
364363
}
365364

@@ -400,7 +399,7 @@ rb_concurrent_set_delete_by_identity(VALUE set_obj, VALUE key)
400399

401400
while (true) {
402401
struct concurrent_set_entry *entry = &set->entries[idx];
403-
VALUE curr_key = RUBY_ATOMIC_VALUE_LOAD(entry->key);
402+
VALUE curr_key = entry->key;
404403

405404
switch (curr_key) {
406405
case CONCURRENT_SET_EMPTY:

0 commit comments

Comments
 (0)