diff options
| author | Garrett D'Amore <garrett@damore.org> | 2025-01-01 17:06:39 -0800 |
|---|---|---|
| committer | Garrett D'Amore <garrett@damore.org> | 2025-01-01 17:06:39 -0800 |
| commit | f7bf24f429cbc488b861ab1b1e4cf1983af56295 (patch) | |
| tree | 2196361a906bd5db1148c0e177d69854a99b7b58 | |
| parent | ee5c8437f8c2a811c0eaef9b00c149b93c095391 (diff) | |
| download | nng-f7bf24f429cbc488b861ab1b1e4cf1983af56295.tar.gz nng-f7bf24f429cbc488b861ab1b1e4cf1983af56295.tar.bz2 nng-f7bf24f429cbc488b861ab1b1e4cf1983af56295.zip | |
api: Remove the NNG_FLAG_ALLOC
This flag failed to provide real zero copy that it was intended for,
and it also involved extra allocations. Further, the API for it was
brittle and error prone.
Modern code should just work directly with nng_msg structures.
| -rw-r--r-- | demo/reqrep/reqrep.c | 79 | ||||
| -rw-r--r-- | docs/ref/api/memory.md | 4 | ||||
| -rw-r--r-- | docs/ref/api/sock.md | 28 | ||||
| -rw-r--r-- | docs/ref/migrate/nanomsg.md | 2 | ||||
| -rw-r--r-- | docs/ref/migrate/nng1.md | 9 | ||||
| -rw-r--r-- | docs/ref/xref.md | 1 | ||||
| -rw-r--r-- | include/nng/nng.h | 19 | ||||
| -rw-r--r-- | src/core/sock_test.c | 24 | ||||
| -rw-r--r-- | src/nng.c | 35 | ||||
| -rw-r--r-- | src/sp/nonblock_test.c | 14 |
10 files changed, 66 insertions, 149 deletions
diff --git a/demo/reqrep/reqrep.c b/demo/reqrep/reqrep.c index 5eb96266..9d7110f7 100644 --- a/demo/reqrep/reqrep.c +++ b/demo/reqrep/reqrep.c @@ -29,28 +29,6 @@ #define SERVER "server" #define DATECMD 1 -#define PUT64(ptr, u) \ - do { \ - (ptr)[0] = (uint8_t) (((uint64_t) (u)) >> 56); \ - (ptr)[1] = (uint8_t) (((uint64_t) (u)) >> 48); \ - (ptr)[2] = (uint8_t) (((uint64_t) (u)) >> 40); \ - (ptr)[3] = (uint8_t) (((uint64_t) (u)) >> 32); \ - (ptr)[4] = (uint8_t) (((uint64_t) (u)) >> 24); \ - (ptr)[5] = (uint8_t) (((uint64_t) (u)) >> 16); \ - (ptr)[6] = (uint8_t) (((uint64_t) (u)) >> 8); \ - (ptr)[7] = (uint8_t) ((uint64_t) (u)); \ - } while (0) - -#define GET64(ptr, v) \ - v = (((uint64_t) ((uint8_t) (ptr)[0])) << 56) + \ - (((uint64_t) ((uint8_t) (ptr)[1])) << 48) + \ - (((uint64_t) ((uint8_t) (ptr)[2])) << 40) + \ - (((uint64_t) ((uint8_t) (ptr)[3])) << 32) + \ - (((uint64_t) ((uint8_t) (ptr)[4])) << 24) + \ - (((uint64_t) ((uint8_t) (ptr)[5])) << 16) + \ - (((uint64_t) ((uint8_t) (ptr)[6])) << 8) + \ - (((uint64_t) (uint8_t) (ptr)[7])) - void fatal(const char *func, int rv) { @@ -88,36 +66,34 @@ server(const char *url) nng_listener_start(listener, 0); for (;;) { - char *buf = NULL; - size_t sz; uint64_t val; + nng_msg *msg; count++; - if ((rv = nng_recv(sock, &buf, &sz, NNG_FLAG_ALLOC)) != 0) { + if ((rv = nng_recvmsg(sock, &msg, 0)) != 0) { fatal("nng_recv", rv); } - if ((sz == sizeof(uint64_t)) && - ((GET64(buf, val)) == DATECMD)) { + if ((nng_msg_trim_u64(msg, &val) == 0) && (val == DATECMD)) { time_t now; printf("SERVER: RECEIVED DATE REQUEST\n"); now = time(&now); if (count == 6) { printf("SERVER: SKIP SENDING REPLY\n"); - nng_free(buf, sz); + nng_msg_free(msg); continue; } printf("SERVER: SENDING DATE: "); showdate(now); - // Reuse the buffer. We know it is big enough. - PUT64(buf, (uint64_t) now); - rv = nng_send(sock, buf, sz, NNG_FLAG_ALLOC); + // Reuse the message. We know it is big enough. + nng_msg_append_u64(msg, now); + rv = nng_sendmsg(sock, msg, 0); if (rv != 0) { fatal("nng_send", rv); } - continue; + } else { + // Unrecognized command, so toss the message. + nng_msg_free(msg); } - // Unrecognized command, so toss the buffer. - nng_free(buf, sz); } } @@ -128,12 +104,8 @@ client(const char *url) nng_dialer dialer; int rv; size_t sz; - char *buf = NULL; - uint8_t cmd[sizeof(uint64_t)]; int sleep = 0; - PUT64(cmd, DATECMD); - if ((rv = nng_init(NULL)) != 0) { fatal("nng_init", rv); } @@ -151,22 +123,27 @@ client(const char *url) while (1) { + nng_msg *msg; + uint64_t now; + if ((rv = nng_msg_alloc(&msg, 0)) != 0) { + fatal("nng_msg_alloc", rv); + } + if ((rv = nng_msg_append_u64(msg, DATECMD)) != 0) { + fatal("nng_msg_append", rv); + } printf("CLIENT: SENDING DATE REQUEST\n"); - if ((rv = nng_send(sock, cmd, sizeof(cmd), 0)) != 0) { - fatal("nng_send", rv); + if ((rv = nng_sendmsg(sock, msg, 0)) != 0) { + fatal("nng_sendmsg", rv); } - if ((rv = nng_recv(sock, &buf, &sz, NNG_FLAG_ALLOC)) != 0) { - fatal("nng_recv", rv); + if ((rv = nng_recvmsg(sock, &msg, 0)) != 0) { + fatal("nng_recvmsg", rv); } - - if (sz == sizeof(uint64_t)) { - uint64_t now; - GET64(buf, now); - printf("CLIENT: RECEIVED DATE: "); - showdate((time_t) now); - } else { - printf("CLIENT: GOT WRONG SIZE!\n"); + if ((rv = nng_msg_trim_u64(msg, &now)) != 0) { + fatal("nng_msg_trim_u64", rv); } + nng_msg_free(msg); + printf("CLIENT: RECEIVED DATE: "); + showdate((time_t) now); nng_msleep(sleep); sleep++; if (sleep == 4) { @@ -174,8 +151,6 @@ client(const char *url) } } - // This assumes that buf is ASCIIZ (zero terminated). - nng_free(buf, sz); nng_socket_close(sock); return (0); } diff --git a/docs/ref/api/memory.md b/docs/ref/api/memory.md index c5ecb23a..d8c5ecf9 100644 --- a/docs/ref/api/memory.md +++ b/docs/ref/api/memory.md @@ -19,9 +19,7 @@ Note that the memory may have random data in it, just like with `malloc`. If memory cannot be allocated for any reason, then `NULL` will be returned. Applications that experience this should treat this like [`NNG_ENOMEM`]. -Memory returned by `nng_alloc` can be used to hold message buffers, in which -case it can be directly passed to [`nng_send`] using the flag `NNG_FLAG_ALLOC`. -Alternatively, it can be freed when no longer needed using [`nng_free`]. +Memory returned by `nng_alloc` should be freed when no longer needed using [`nng_free`]. > [!IMPORTANT] > Do not use the system `free` function (or the C++ `delete` operator) to release this memory. diff --git a/docs/ref/api/sock.md b/docs/ref/api/sock.md index 3ea056fb..ec6b20e1 100644 --- a/docs/ref/api/sock.md +++ b/docs/ref/api/sock.md @@ -177,13 +177,6 @@ made up of zero or more of the following values: If the socket cannot accept more data at this time, it does not block, but returns immediately with a status of [`NNG_EAGAIN`]. If this flag is absent, the function will wait until data can be sent. -- {{i:`NNG_FLAG_ALLOC`}}: <a name="NNG_FLAG_ALLOC"></a> - The _data_ was allocated using [`nng_alloc`] or was obtained from a call to [`nng_recv`] also with - the `NNG_FLAG_ALLOC` flag. If this function succeeds, then it will dispose of the _data_, deallocating it - once the transmission is complete. If this function returns a non-zero status, the caller retains the responsibility - of disposing the data. The benefit of this flag is that it can eliminate a data copy and allocation. Without the flag - the socket will make a duplicate copy of _data_ for use by the operation, before returning to the caller. - > [!NOTE] > Regardless of the presence or absence of `NNG_FLAG_NONBLOCK`, there may > be queues between the sender and the receiver. @@ -191,11 +184,6 @@ made up of zero or more of the following values: > Finally, with some protocols, the semantic is implicitly `NNG_FLAG_NONBLOCK`, > such as with [PUB][pub] sockets, which are best-effort delivery only. -> [!IMPORTANT] -> When using `NNG_FLAG_ALLOC`, it is important that the value of _size_ match the actual allocated size of the data. -> Using an incorrect size results in unspecified behavior, which may include heap corruption, program crashes, -> or other undesirable effects. - ### nng_sendmsg The `nng_sendmsg` function sends the _msg_ over the socket _s_. @@ -248,8 +236,7 @@ messages over the socket _s_. The differences in their behaviors are as follows. ### nng_recv The `nng_recv` function is the simplest to use, but is the least efficient. -It receives the content in _data_, as a message size (in bytes) of up to the value stored in _sizep_, -unless the `NNG_FLAG_ALLOC` flag is set in _flags_ (see below.) +It receives the content in _data_, as a message size (in bytes) of up to the value stored in _sizep_. Upon success, the size of the message received will be stored in _sizep_. @@ -259,17 +246,6 @@ The _flags_ is a bit mask made up of zero or more of the following values: If the socket has no messages pending for reception at this time, it does not block, but returns immediately with a status of [`NNG_EAGAIN`]. If this flag is absent, the function will wait until data can be received. -- {{i:`NNG_FLAG_ALLOC`}}: - Instead of receiving the message into _data_, a new buffer will be allocated exactly large enough to hold - the message. A pointer to that buffer will be stored at the location specified by _data_. This provides a form - of zero-copy operation. The caller should dispose of the buffer using [`nng_free`] or by sending using - [`nng_send`] with the [`NNG_FLAG_ALLOC`] flag. - -> [!IMPORTANT] -> When using `NNG_FLAG_ALLOC`, it is important that the value of _size_ match the actual allocated size of the data. -> Using an incorrect size results in unspecified behavior, which may include heap corruption, program crashes, -> or other undesirable effects. - ### nng_recvmsg The `nng_recvmsg` function receives a message and stores a pointer to the [`nng_msg`] for that message in _msgp_. @@ -279,7 +255,7 @@ has no messages available to receive. In such a case, it will return [`NNG_EAGAI > [!TIP] > This function is preferred over [`nng_recv`], as it gives access to the message structure and eliminates both -> a data copy and allocation, even when `nng_recv` is using `NNG_FLAG_ALLOC`. +> a data copy and allocation. ### nng_recv_aio diff --git a/docs/ref/migrate/nanomsg.md b/docs/ref/migrate/nanomsg.md index 47f82a9c..78eed618 100644 --- a/docs/ref/migrate/nanomsg.md +++ b/docs/ref/migrate/nanomsg.md @@ -56,7 +56,7 @@ NNG approach to messages. Likewise there is no `struct nn_cmsghdr` equivalent. | `nn_get_statistic` | [`nng_stats_get`] | The statistics in NNG are completely different, with different semantics and no stability guarantees. | | `NN_POLLIN` | None | Used only with `nn_poll`. | | `NN_POLLOUT` | None | Used only with `nn_poll`. | -| `NN_MSG` | [`NNG_FLAG_ALLOC`] | See `nng_send` and `nng_recv` for details. | +| `NN_MSG` | [`nng_sendmsg`], [`nng_recvmsg`] | There are differences as a separate [`nng_msg`] structure is involved. | | `NN_CMSG_ALIGN` | None | | `NN_CMSG_FIRSTHDR` | None | | `NN_CMSG_NXTHDR` | None | diff --git a/docs/ref/migrate/nng1.md b/docs/ref/migrate/nng1.md index 65f1a81e..8e418468 100644 --- a/docs/ref/migrate/nng1.md +++ b/docs/ref/migrate/nng1.md @@ -40,6 +40,15 @@ be supplied should avoid surprises later as new versions of protocols are added. Additionally, the header files for protocols are now empty, as all of their content has been moved to `nng/nng.h`. Please remove `#include` references to protocol headers as we anticipate removing them in the future. +## NNG_FLAG_ALLOC Removed + +The `NNG_FLAG_ALLOC` flag that allowed a zero copy semantic with [`nng_send`] and [`nng_recv`] is removed. +This was implemented mostly to aid legacy nanomsg applications, and it was both error prone and still a bit +suboptimal in terms of performance. + +Modern code should use one of [`nng_sendmsg`], [`nng_recvmsg`], [`nng_send_aio`], or [`nng_recv_aio`] to get the maximum performance benefit. +Working directly with [`nng_msg`] structures gives more control, reduces copies, and reduces allocation activity. + ## New AIO Error Code NNG_ESTOPPED When an operation fails with [`NNG_ESTOPPED`], it means that the associated [`nni_aio`] object has diff --git a/docs/ref/xref.md b/docs/ref/xref.md index 88958feb..2d98e1d4 100644 --- a/docs/ref/xref.md +++ b/docs/ref/xref.md @@ -282,7 +282,6 @@ [`NNG_UNIT_MESSAGES`]: /api/stats.md#statistic-units [`NNG_UNIT_MILLIS`]: /api/stats.md#statistic-units [`NNG_UNIT_EVENTS`]: /api/stats.md#statistic-units -[`NNG_FLAG_ALLOC`]: /TODO.md [`NNG_FLAG_NONBLOCK`]: /TODO.md [`NNG_OPT_LISTEN_FD`]: /api/streams.md#socket-activation [`NNG_OPT_MAXTTL`]: /TODO.md diff --git a/include/nng/nng.h b/include/nng/nng.h index a3dafb7a..664da2d5 100644 --- a/include/nng/nng.h +++ b/include/nng/nng.h @@ -387,19 +387,12 @@ NNG_DECL const char *nng_strerror(int); // this function may (will!) return before any receiver has actually // received the data. The return value will be zero to indicate that the // socket has accepted the entire data for send, or an errno to indicate -// failure. The flags may include NNG_FLAG_NONBLOCK or NNG_FLAG_ALLOC. -// If the flag includes NNG_FLAG_ALLOC, then the function will call -// nng_free() on the supplied pointer & size on success. (If the call -// fails then the memory is not freed.) +// failure. The flags may include NNG_FLAG_NONBLOCK. NNG_DECL int nng_send(nng_socket, void *, size_t, int); // nng_recv receives message data into the socket, up to the supplied size. // The actual size of the message data will be written to the value pointed -// to by size. The flags may include NNG_FLAG_NONBLOCK and NNG_FLAG_ALLOC. -// If NNG_FLAG_ALLOC is supplied then the library will allocate memory for -// the caller. In that case the pointer to the allocated will be stored -// instead of the data itself. The caller is responsible for freeing the -// associated memory with nng_free(). +// to by size. The flags may include NNG_FLAG_NONBLOCK. NNG_DECL int nng_recv(nng_socket, void *, size_t *, int); // nng_sendmsg is like nng_send, but offers up a message structure, which @@ -488,10 +481,9 @@ NNG_DECL int nng_ctx_set_ms(nng_ctx, const char *, nng_duration); // specific API. NNG_DECL void *nng_alloc(size_t); -// nng_free is used to free memory allocated with nng_alloc, which includes -// memory allocated by nng_recv() when the NNG_FLAG_ALLOC message is supplied. -// As the application is required to keep track of the size of memory, this -// is probably less convenient for general uses than the C library malloc and +// nng_free is used to free memory allocated with nng_alloc. As the +// application is required to keep track of the size of memory, this is +// probably less convenient for general uses than the C library malloc and // calloc. NNG_DECL void nng_free(void *, size_t); @@ -711,7 +703,6 @@ NNG_DECL nng_dialer nng_pipe_dialer(nng_pipe); NNG_DECL nng_listener nng_pipe_listener(nng_pipe); // Flags. -#define NNG_FLAG_ALLOC 1u // Recv to allocate receive buffer #define NNG_FLAG_NONBLOCK 2u // Non-blocking operations // Options. diff --git a/src/core/sock_test.c b/src/core/sock_test.c index e311634d..f6f1bd7d 100644 --- a/src/core/sock_test.c +++ b/src/core/sock_test.c @@ -102,8 +102,8 @@ test_send_recv(void) int len; size_t sz; nng_duration to = 3000; // 3 seconds - char *buf; - char *a = "inproc://t1"; + char *a = "inproc://t1"; + char rxbuf[32]; NUTS_OPEN(s1); NUTS_OPEN(s2); @@ -124,11 +124,10 @@ test_send_recv(void) NUTS_PASS(nng_dial(s2, a, NULL, 0)); NUTS_PASS(nng_send(s1, "abc", 4, 0)); - NUTS_PASS(nng_recv(s2, &buf, &sz, NNG_FLAG_ALLOC)); - NUTS_TRUE(buf != NULL); + sz = sizeof(rxbuf); + NUTS_PASS(nng_recv(s2, rxbuf, &sz, 0)); NUTS_TRUE(sz == 4); - NUTS_TRUE(memcmp(buf, "abc", 4) == 0); - nng_free(buf, sz); + NUTS_TRUE(memcmp(rxbuf, "abc", 4) == 0); NUTS_CLOSE(s1); NUTS_CLOSE(s2); @@ -142,7 +141,7 @@ test_send_recv_zero_length(void) int len; size_t sz; nng_duration to = 3000; // 3 seconds - char *buf; + char buf[32]; char *a = "inproc://send-recv-zero-length"; NUTS_OPEN(s1); @@ -164,10 +163,9 @@ test_send_recv_zero_length(void) NUTS_PASS(nng_dial(s2, a, NULL, 0)); NUTS_PASS(nng_send(s1, "", 0, 0)); - NUTS_PASS(nng_recv(s2, &buf, &sz, NNG_FLAG_ALLOC)); - NUTS_TRUE(buf == NULL); + sz = sizeof(buf); + NUTS_PASS(nng_recv(s2, buf, &sz, 0)); NUTS_TRUE(sz == 0); - nng_free(buf, sz); NUTS_CLOSE(s1); NUTS_CLOSE(s2); @@ -186,7 +184,7 @@ test_connection_refused(void) void test_late_connection(void) { - char *buf; + char buf[32]; size_t sz; nng_socket s1; nng_socket s2; @@ -202,10 +200,10 @@ test_late_connection(void) NUTS_PASS(nng_listen(s2, a, NULL, 0)); nng_msleep(100); NUTS_PASS(nng_send(s1, "abc", 4, 0)); - NUTS_PASS(nng_recv(s2, &buf, &sz, NNG_FLAG_ALLOC)); + sz = sizeof(buf); + NUTS_PASS(nng_recv(s2, &buf, &sz, 0)); NUTS_TRUE(sz == 4); NUTS_TRUE(memcmp(buf, "abc", 4) == 0); - nng_free(buf, sz); NUTS_CLOSE(s1); NUTS_CLOSE(s2); @@ -79,35 +79,12 @@ nng_recv(nng_socket s, void *buf, size_t *szp, int flags) // Note that while it would be nice to make this a zero copy operation, // its not normally possible if a size was specified. - if ((rv = nng_recvmsg(s, &msg, flags & ~(NNG_FLAG_ALLOC))) != 0) { + if ((rv = nng_recvmsg(s, &msg, flags)) != 0) { return (rv); } - if (!(flags & NNG_FLAG_ALLOC)) { - memcpy(buf, nng_msg_body(msg), - *szp > nng_msg_len(msg) ? nng_msg_len(msg) : *szp); - *szp = nng_msg_len(msg); - } else { - // We'd really like to avoid a separate data copy, but since - // we have allocated messages with headroom, we can't really - // make free() work on the base pointer. We'd have to have - // some other API for this. Folks that want zero copy had - // better use nng_recvmsg() instead. - void *nbuf; - - if (nng_msg_len(msg) != 0) { - if ((nbuf = nni_alloc(nng_msg_len(msg))) == NULL) { - nng_msg_free(msg); - return (NNG_ENOMEM); - } - - *(void **) buf = nbuf; - memcpy(nbuf, nni_msg_body(msg), nni_msg_len(msg)); - *szp = nng_msg_len(msg); - } else { - *(void **) buf = NULL; - *szp = 0; - } - } + memcpy(buf, nng_msg_body(msg), + *szp > nng_msg_len(msg) ? nng_msg_len(msg) : *szp); + *szp = nng_msg_len(msg); nni_msg_free(msg); return (0); } @@ -159,10 +136,6 @@ nng_send(nng_socket s, void *buf, size_t len, int flags) if ((rv = nng_sendmsg(s, msg, flags)) != 0) { // If nng_sendmsg() succeeded, then it took ownership. nng_msg_free(msg); - } else { - if (flags & NNG_FLAG_ALLOC) { - nni_free(buf, len); - } } return (rv); } diff --git a/src/sp/nonblock_test.c b/src/sp/nonblock_test.c index 7b2251c4..c06fa64d 100644 --- a/src/sp/nonblock_test.c +++ b/src/sp/nonblock_test.c @@ -1,5 +1,5 @@ // -// Copyright 2024 Staysail Systems, Inc. <info@staysail.tech> +// Copyright 2025 Staysail Systems, Inc. <info@staysail.tech> // Copyright 2018 Capitar IT Group BV <info@capitar.com> // // This software is supplied under the terms of the MIT License, a @@ -46,8 +46,7 @@ repthr(void *arg) for (;;) { fd_set fset; struct timeval tmo; - char *msgbuf; - size_t msglen; + nng_msg *msg; FD_ZERO(&fset); FD_SET(fd, &fset); @@ -59,14 +58,13 @@ repthr(void *arg) for (;;) { int rv; - rv = nng_recv(rep, &msgbuf, &msglen, - NNG_FLAG_NONBLOCK | NNG_FLAG_ALLOC); + rv = nng_recvmsg(rep, &msg, NNG_FLAG_NONBLOCK); if (rv != 0) { return; } - nng_free(msgbuf, msglen); - int ok = 0; - rv = nng_send(rep, &ok, 4, NNG_FLAG_NONBLOCK); + nng_msg_clear(msg); + nng_msg_append_u32(msg, 0); + rv = nng_sendmsg(rep, msg, NNG_FLAG_NONBLOCK); if (rv == NNG_ECLOSED) { return; } |
