Skip to content

Replace __sync reference-count atomics with std::atomic #399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DTW-Thalion wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Replace __sync reference-count atomics with std::atomic #399

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
79 changes: 52 additions & 27 deletions arc.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <atomic>
#include <vector>
#include <tsl/robin_map.h>
#import "lock.h"
Expand Down Expand Up @@ -270,19 +271,18 @@ static TLS_CALLBACK(cleanupPools)(struct arc_tls* tls)

extern "C" OBJC_PUBLIC size_t object_getRetainCount_np(id obj)
{
uintptr_t *refCount = ((uintptr_t*)obj) - 1;
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);

@davidchisnall davidchisnall Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use std::atomic instead of builtins?

@DTW-Thalion DTW-Thalion Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Now uses std::atomic_ref<uintptr_t> over the count word instead of the builtins, keeping the same orderings: a relaxed load seeds each loop, compare_exchange_weak is relaxed on the retain and weak-flag paths and release on the decrement, and there's an acquire fence before -dealloc. All 194 tests still pass, along with the retain/release balance and weak-race stresses. Should mention that this seems to work with C++20 but may not work on earlier versions.

The other option was reinterpret_cast<std::atomic<uintptr_t>(refcount) which has the upside of working on any C++11+ standard library. We can revisit that if you prefer. The downside is this is technically UB - accessing an object that wasn't created as a std::atomic through an atomic pointer. So I went with the standard way at the expense of a libc++ >= 19.

@davidchisnall davidchisnall Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was more thinking of chaning the previous line to:

	auto *refCount = reinterpret_cast<std::atomic<uintptr_t>*>(obj) - 1;

@DTW-Thalion DTW-Thalion Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, that's cleaner. Switched all five sites to reinterpret_cast<std::atomic<uintptr_t>*>(obj) - 1 and operate on it directly. Same orderings; all 194 tests plus the retain/release balance and weak-race stresses still pass.

auto *refCount = reinterpret_cast<std::atomic<uintptr_t>*>(obj) - 1;
uintptr_t refCountVal = refCount->load(std::memory_order_relaxed);
size_t realCount = refCountVal & refcount_mask;
return realCount == refcount_mask ? 0 : realCount + 1;
}

static id retain_fast(id obj, BOOL isWeak)
{
uintptr_t *refCount = ((uintptr_t*)obj) - 1;
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);
uintptr_t newVal = refCountVal;
do {
refCountVal = newVal;
auto *refCount = reinterpret_cast<std::atomic<uintptr_t>*>(obj) - 1;
uintptr_t refCountVal = refCount->load(std::memory_order_relaxed);
for (;;)
{
size_t realCount = refCountVal & refcount_mask;
// If this object's reference count is already less than 0, then
// this is a spurious retain. This can happen when one thread is
Expand Down Expand Up @@ -310,9 +310,16 @@ static id retain_fast(id obj, BOOL isWeak)
realCount++;
realCount |= refCountVal & weak_mask;
uintptr_t updated = (uintptr_t)realCount;
newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated);
} while (newVal != refCountVal);
return obj;
// Acquire/release on the exchange so reference-count updates are
// ordered against each other on weakly-ordered targets. On a failed
// exchange refCountVal is refreshed with the current value.
if (refCount->compare_exchange_weak(refCountVal, updated,
std::memory_order_acq_rel,
std::memory_order_acquire))
{
return obj;
}
}
}

extern "C" OBJC_PUBLIC id objc_retain_fast_np(id obj)
Expand Down Expand Up @@ -355,13 +362,12 @@ static inline id retain(id obj, BOOL isWeak)

extern "C" OBJC_PUBLIC BOOL objc_release_fast_no_destroy_np(id obj)
{
uintptr_t *refCount = ((uintptr_t*)obj) - 1;
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);
uintptr_t newVal = refCountVal;
auto *refCount = reinterpret_cast<std::atomic<uintptr_t>*>(obj) - 1;
uintptr_t refCountVal = refCount->load(std::memory_order_relaxed);
bool isWeak;
bool shouldFree;
do {
refCountVal = newVal;
for (;;)
{
size_t realCount = refCountVal & refcount_mask;
// If the reference count is saturated or deallocating, don't decrement it.
if (realCount >= refcount_max)
Expand All @@ -373,11 +379,23 @@ static inline id retain(id obj, BOOL isWeak)
shouldFree = realCount == -1;
realCount |= refCountVal & weak_mask;
uintptr_t updated = (uintptr_t)realCount;
newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated);
} while (newVal != refCountVal);

// Release ordering on the decrement so that writes made through the
// references being dropped are visible to whichever thread performs
// the final release. refCountVal is refreshed on a failed exchange.
if (refCount->compare_exchange_weak(refCountVal, updated,
std::memory_order_release,
std::memory_order_relaxed))
{
break;
}
}

if (shouldFree)
{
// Acquire fence pairing with the release above, so this thread sees
// every write made under the object's prior references before it
// runs -dealloc.
std::atomic_thread_fence(std::memory_order_acquire);
if (isWeak)
{
if (!objc_delete_weak_refs(obj))
Expand Down Expand Up @@ -796,11 +814,10 @@ static BOOL setObjectHasWeakRefs(id obj)
Class cls = isGlobalObject ? Nil : obj->isa;
if (obj && cls && objc_test_class_flag(cls, objc_class_flag_fast_arc))
{
uintptr_t *refCount = ((uintptr_t*)obj) - 1;
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);
uintptr_t newVal = refCountVal;
do {
refCountVal = newVal;
auto *refCount = reinterpret_cast<std::atomic<uintptr_t>*>(obj) - 1;
uintptr_t refCountVal = refCount->load(std::memory_order_relaxed);
for (;;)
{
size_t realCount = refCountVal & refcount_mask;
// If this object has already been deallocated (or is in the
// process of being deallocated) then don't bother storing it.
Expand All @@ -826,8 +843,16 @@ static BOOL setObjectHasWeakRefs(id obj)
// reference and so it shouldn't be possible to deallocate it
// while we're assigning it.
uintptr_t updated = ((uintptr_t)realCount | weak_mask);
newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated);
} while (newVal != refCountVal);
// Acquire/release on the exchange, matching the other
// reference-count updates. The weak-ref lock, held here, orders
// the weak-table entry we publish next.
if (refCount->compare_exchange_weak(refCountVal, updated,

@davidchisnall davidchisnall Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As above.

std::memory_order_acq_rel,
std::memory_order_acquire))
{
break;
}
}
}
return isGlobalObject;
}
Expand Down Expand Up @@ -907,8 +932,8 @@ static BOOL setObjectHasWeakRefs(id obj)
if (objc_test_class_flag(classForObject(obj), objc_class_flag_fast_arc))
{
// Don't proceed if the object isn't deallocating.
uintptr_t *refCount = ((uintptr_t*)obj) - 1;
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);
auto *refCount = reinterpret_cast<std::atomic<uintptr_t>*>(obj) - 1;
uintptr_t refCountVal = refCount->load(std::memory_order_relaxed);

@davidchisnall davidchisnall Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one is okay, but only because we're relying on the caller already having done the synchronisation to ensure that this cannot be concurrently modified.

size_t realCount = refCountVal & refcount_mask;
if (realCount != refcount_mask)
{
Expand Down
Loading