Skip to content
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.

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

performance: reference counters can use relaxed order when incrementing #1973

Merged
merged 1 commit into from
Dec 7, 2024
Merged
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
7 changes: 5 additions & 2 deletions src/core/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,6 @@ extern void nni_atomic_sub64(nni_atomic_u64 *, uint64_t);
extern uint64_t nni_atomic_get64(nni_atomic_u64 *);
extern void nni_atomic_set64(nni_atomic_u64 *, uint64_t);
extern uint64_t nni_atomic_swap64(nni_atomic_u64 *, uint64_t);
extern uint64_t nni_atomic_dec64_nv(nni_atomic_u64 *);
extern void nni_atomic_inc64(nni_atomic_u64 *);

// nni_atomic_cas64 is a compare and swap. The second argument is the
// value to compare against, and the third is the new value. Returns
Expand All @@ -218,6 +216,11 @@ extern void nni_atomic_sub(nni_atomic_int *, int);
extern int nni_atomic_get(nni_atomic_int *);
extern void nni_atomic_set(nni_atomic_int *, int);
extern int nni_atomic_swap(nni_atomic_int *, int);

// These versions are tuned for use as reference
// counters. Relaxed order when possible to increase
// reference count, acquire-release order for dropping
// it (where we need to check the value).
extern int nni_atomic_dec_nv(nni_atomic_int *);
extern void nni_atomic_dec(nni_atomic_int *);
extern void nni_atomic_inc(nni_atomic_int *);
Expand Down
7 changes: 4 additions & 3 deletions src/platform/posix/posix_atomic.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ nni_atomic_swap(nni_atomic_int *v, int i)
void
nni_atomic_inc(nni_atomic_int *v)
{
atomic_fetch_add(&v->v, 1);
atomic_fetch_add_explicit(&v->v, 1, memory_order_relaxed);
}

void
Expand All @@ -117,7 +117,7 @@ nni_atomic_dec(nni_atomic_int *v)
int
nni_atomic_dec_nv(nni_atomic_int *v)
{
return (atomic_fetch_sub(&v->v, 1) - 1);
return (atomic_fetch_sub_explicit(&v->v, 1, memory_order_acq_rel) - 1);
}

bool
Expand Down Expand Up @@ -319,10 +319,11 @@ nni_atomic_get(nni_atomic_int *v)
{
return (__atomic_load_n(&v->v, __ATOMIC_SEQ_CST));
}

void
nni_atomic_inc(nni_atomic_int *v)
{
__atomic_add_fetch(&v->v, 1, __ATOMIC_SEQ_CST);
__atomic_add_fetch(&v->v, 1, __ATOMIC_RELAXED);
}

void
Expand Down
8 changes: 4 additions & 4 deletions src/platform/posix/posix_ipc.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@

struct nni_ipc_conn {
nng_stream stream;
nni_posix_pfd * pfd;
nni_posix_pfd *pfd;
nni_list readq;
nni_list writeq;
bool closed;
nni_mtx mtx;
nni_aio * dial_aio;
nni_aio *dial_aio;
nni_ipc_dialer *dialer;
nng_sockaddr sa;
nni_reap_node reap;
Expand All @@ -39,7 +39,7 @@ struct nni_ipc_dialer {
bool closed;
nni_mtx mtx;
nng_sockaddr sa;
nni_atomic_u64 ref;
nni_atomic_int ref;
nni_atomic_bool fini;
};

Expand All @@ -51,4 +51,4 @@ extern void nni_posix_ipc_dialer_rele(nni_ipc_dialer *);

#endif // NNG_PLATFORM_POSIX

#endif // PLATFORM_POSIX_IPC_H
#endif // PLATFORM_POSIX_IPC_H
10 changes: 5 additions & 5 deletions src/platform/posix/posix_ipcdial.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2023 Staysail Systems, Inc. <[email protected]>
// Copyright 2024 Staysail Systems, Inc. <[email protected]>
// Copyright 2018 Capitar IT Group BV <[email protected]>
// Copyright 2019 Devolutions <[email protected]>
//
Expand Down Expand Up @@ -69,7 +69,7 @@ ipc_dialer_free(void *arg)
void
nni_posix_ipc_dialer_rele(ipc_dialer *d)
{
if (((nni_atomic_dec64_nv(&d->ref)) != 0) ||
if (((nni_atomic_dec_nv(&d->ref)) != 0) ||
(!nni_atomic_get_bool(&d->fini))) {
return;
}
Expand Down Expand Up @@ -176,7 +176,7 @@ ipc_dialer_dial(void *arg, nni_aio *aio)
return;
}

nni_atomic_inc64(&d->ref);
nni_atomic_inc(&d->ref);

if ((rv = nni_posix_ipc_alloc(&c, &d->sa, d)) != 0) {
(void) close(fd);
Expand Down Expand Up @@ -323,8 +323,8 @@ nni_ipc_dialer_alloc(nng_stream_dialer **dp, const nng_url *url)
d->sd.sd_get = ipc_dialer_get;
d->sd.sd_set = ipc_dialer_set;
nni_atomic_init_bool(&d->fini);
nni_atomic_init64(&d->ref);
nni_atomic_inc64(&d->ref);
nni_atomic_init(&d->ref);
nni_atomic_inc(&d->ref);

*dp = (void *) d;
return (0);
Expand Down
8 changes: 4 additions & 4 deletions src/platform/posix/posix_tcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@

struct nni_tcp_conn {
nng_stream stream;
nni_posix_pfd * pfd;
nni_posix_pfd *pfd;
nni_list readq;
nni_list writeq;
bool closed;
nni_mtx mtx;
nni_aio * dial_aio;
nni_aio *dial_aio;
nni_tcp_dialer *dialer;
nni_reap_node reap;
};
Expand All @@ -36,7 +36,7 @@ struct nni_tcp_dialer {
struct sockaddr_storage src;
size_t srclen;
nni_mtx mtx;
nni_atomic_u64 ref;
nni_atomic_int ref;
nni_atomic_bool fini;
};

Expand All @@ -45,4 +45,4 @@ extern void nni_posix_tcp_init(nni_tcp_conn *, nni_posix_pfd *);
extern void nni_posix_tcp_start(nni_tcp_conn *, int, int);
extern void nni_posix_tcp_dialer_rele(nni_tcp_dialer *);

#endif // PLATFORM_POSIX_TCP_H
#endif // PLATFORM_POSIX_TCP_H
8 changes: 4 additions & 4 deletions src/platform/posix/posix_tcpdial.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ nni_tcp_dialer_init(nni_tcp_dialer **dp)
d->nodelay = true;
nni_aio_list_init(&d->connq);
nni_atomic_init_bool(&d->fini);
nni_atomic_init64(&d->ref);
nni_atomic_inc64(&d->ref);
nni_atomic_init(&d->ref);
nni_atomic_inc(&d->ref);
*dp = d;
return (0);
}
Expand Down Expand Up @@ -87,7 +87,7 @@ nni_tcp_dialer_fini(nni_tcp_dialer *d)
void
nni_posix_tcp_dialer_rele(nni_tcp_dialer *d)
{
if (((nni_atomic_dec64_nv(&d->ref) != 0)) ||
if (((nni_atomic_dec_nv(&d->ref) != 0)) ||
(!nni_atomic_get_bool(&d->fini))) {
return;
}
Expand Down Expand Up @@ -200,7 +200,7 @@ nni_tcp_dial(nni_tcp_dialer *d, const nni_sockaddr *sa, nni_aio *aio)
return;
}

nni_atomic_inc64(&d->ref);
nni_atomic_inc(&d->ref);

if ((rv = nni_posix_tcp_alloc(&c, d)) != 0) {
nni_aio_finish_error(aio, rv);
Expand Down
6 changes: 4 additions & 2 deletions src/platform/windows/win_thread.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2021 Staysail Systems, Inc. <[email protected]>
// Copyright 2024 Staysail Systems, Inc. <[email protected]>
// Copyright 2018 Capitar IT Group BV <[email protected]>
//
// This software is supplied under the terms of the MIT License, a
Expand All @@ -22,6 +22,8 @@ static pfnSetThreadDescription set_thread_desc;
// mingw does not define InterlockedAddNoFence64, use the mingw equivalent
#if defined(__MINGW32__) || defined(__MINGW64__)
#define InterlockedAddNoFence(a, b) __atomic_add_fetch(a, b, __ATOMIC_RELAXED)
#define InterlockedIncrementNoFence(a) \
__atomic_add_fetch(a, 1, __ATOMIC_RELAXED)
#define InterlockedAddNoFence64(a, b) \
__atomic_add_fetch(a, b, __ATOMIC_RELAXED)
#define InterlockedIncrementAcquire64(a) \
Expand Down Expand Up @@ -326,7 +328,7 @@ nni_atomic_init(nni_atomic_int *v)
void
nni_atomic_inc(nni_atomic_int *v)
{
(void) InterlockedIncrementAcquire(&v->v);
(void) InterlockedIncrementNoFence(&v->v);
}

int
Expand Down
10 changes: 5 additions & 5 deletions src/supplemental/http/http_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ struct nng_http_handler {
bool host_ip;
bool tree;
bool tree_exclusive;
nni_atomic_u64 ref;
nni_atomic_int ref;
nni_atomic_bool busy;
size_t maxbody;
bool getbody;
Expand Down Expand Up @@ -107,8 +107,8 @@ nni_http_handler_init(
if ((h = NNI_ALLOC_STRUCT(h)) == NULL) {
return (NNG_ENOMEM);
}
nni_atomic_init64(&h->ref);
nni_atomic_inc64(&h->ref);
nni_atomic_init(&h->ref);
nni_atomic_inc(&h->ref);

// Default for HTTP is /. But remap it to "" for ease of matching.
if ((uri == NULL) || (strlen(uri) == 0) || (strcmp(uri, "/") == 0)) {
Expand Down Expand Up @@ -137,7 +137,7 @@ nni_http_handler_init(
void
nni_http_handler_fini(nni_http_handler *h)
{
if (nni_atomic_dec64_nv(&h->ref) != 0) {
if (nni_atomic_dec_nv(&h->ref) != 0) {
return;
}
if (h->dtor != NULL) {
Expand Down Expand Up @@ -735,7 +735,7 @@ http_sconn_rxdone(void *arg)

// Set a reference -- this because the callback may be running
// asynchronously even after it gets removed from the server.
nni_atomic_inc64(&h->ref);
nni_atomic_inc(&h->ref);

// Documented that we call this on behalf of the callback.
if (nni_aio_begin(sc->cbaio) != 0) {
Expand Down
Loading