From dd32e53ad6015b7be97f6ea49d17e8b51c9f4223 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Sat, 25 Feb 2023 18:45:05 +0100 Subject: [PATCH] Add two-step write feature to bytebuf If there is exactly one producer, then it can assume that the remaining space in the buffer will only increase until it write something. This assumption may allow the producer to write to the buffer (up to a known safe size) without any synchronization mechanism, thus allowing to read and write different parts of the buffer in parallel. The producer can then commit the write with lock held, and update its knowledge of the safe empty remaining space. --- app/src/util/bytebuf.c | 28 +++++++++++++++++++++++++ app/src/util/bytebuf.h | 26 ++++++++++++++++++++++++ app/tests/test_bytebuf.c | 44 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/app/src/util/bytebuf.c b/app/src/util/bytebuf.c index d31ada14..ad23a29a 100644 --- a/app/src/util/bytebuf.c +++ b/app/src/util/bytebuf.c @@ -84,3 +84,31 @@ sc_bytebuf_write(struct sc_bytebuf *buf, const uint8_t *from, size_t len) { } buf->head = (buf->head + len) % buf->alloc_size; } + +void +sc_bytebuf_prepare_write(struct sc_bytebuf *buf, const uint8_t *from, + size_t len) { + // *This function MUST NOT access buf->tail (even in assert()).* + // The purpose of this function is to allow a reader and a writer to access + // different parts of the buffer in parallel simultaneously. It is intended + // to be called without lock (only sc_bytebuf_commit_write() is intended to + // be called with lock held). + + assert(len < buf->alloc_size - 1); + + size_t right_len = buf->alloc_size - buf->head; + if (len < right_len) { + right_len = len; + } + + memcpy(buf->data + buf->head, from, right_len); + if (len > right_len) { + memcpy(buf->data, from + right_len, len - right_len); + } +} + +void +sc_bytebuf_commit_write(struct sc_bytebuf *buf, size_t len) { + assert(len <= sc_bytebuf_write_remaining(buf)); + buf->head = (buf->head + len) % buf->alloc_size; +} diff --git a/app/src/util/bytebuf.h b/app/src/util/bytebuf.h index ca502ee9..905cc1fb 100644 --- a/app/src/util/bytebuf.h +++ b/app/src/util/bytebuf.h @@ -25,6 +25,8 @@ sc_bytebuf_init(struct sc_bytebuf *buf, size_t alloc_size); * * The caller must check that len <= buf->len (it is an error to attempt to read * more bytes than available). + * + * This function is guaranteed to not change the head. */ void sc_bytebuf_read(struct sc_bytebuf *buf, uint8_t *to, size_t len); @@ -35,6 +37,8 @@ sc_bytebuf_read(struct sc_bytebuf *buf, uint8_t *to, size_t len); * The caller must check that len <= buf->len (it is an error to attempt to skip * more bytes than available). * + * This function is guaranteed to not change the head. + * * It is equivalent to call sc_bytebuf_read() to some array and discard the * array (but more efficient since there is no copy). */ @@ -51,6 +55,28 @@ sc_bytebuf_skip(struct sc_bytebuf *buf, size_t len); void sc_bytebuf_write(struct sc_bytebuf *buf, const uint8_t *from, size_t len); +/** + * Copy the user-provided array to the bytebuf, but do not advance the cursor + * + * The caller must check that len <= buf->len (it is an error to attempt to + * write more bytes than available). + * + * After this function is called, the write must be committed with + * sc_bytebuf_commit_write(). + * + * The purpose of this mechanism is to acquire a lock only to commit the write, + * but not to perform the actual copy. + */ +void +sc_bytebuf_prepare_write(struct sc_bytebuf *buf, const uint8_t *from, + size_t len); + +/** + * Commit a prepared write + */ +void +sc_bytebuf_commit_write(struct sc_bytebuf *buf, size_t len); + /** * Return the number of bytes which can be read * diff --git a/app/tests/test_bytebuf.c b/app/tests/test_bytebuf.c index 3f0173df..f68c5af9 100644 --- a/app/tests/test_bytebuf.c +++ b/app/tests/test_bytebuf.c @@ -98,6 +98,49 @@ void test_bytebuf_overwrite(void) { sc_bytebuf_destroy(&buf); } +void test_bytebuf_two_steps_write(void) { + struct sc_bytebuf buf; + uint8_t data[20]; + + bool ok = sc_bytebuf_init(&buf, 20); + assert(ok); + + sc_bytebuf_write(&buf, (uint8_t *) "hello ", sizeof("hello ") - 1); + assert(sc_bytebuf_read_remaining(&buf) == 6); + + sc_bytebuf_write(&buf, (uint8_t *) "hello ", sizeof("hello ") - 1); + assert(sc_bytebuf_read_remaining(&buf) == 12); + + sc_bytebuf_prepare_write(&buf, (uint8_t *) "hello ", sizeof("hello ") - 1); + assert(sc_bytebuf_read_remaining(&buf) == 12); // write not committed yet + + sc_bytebuf_read(&buf, data, 9); + assert(!strncmp((char *) data, "hello hel", 3)); + assert(sc_bytebuf_read_remaining(&buf) == 3); + + sc_bytebuf_commit_write(&buf, sizeof("hello ") - 1); + assert(sc_bytebuf_read_remaining(&buf) == 9); + + sc_bytebuf_prepare_write(&buf, (uint8_t *) "world", sizeof("world") - 1); + assert(sc_bytebuf_read_remaining(&buf) == 9); // write not committed yet + + sc_bytebuf_commit_write(&buf, sizeof("world") - 1); + assert(sc_bytebuf_read_remaining(&buf) == 14); + + sc_bytebuf_write(&buf, (uint8_t *) "!", 1); + assert(sc_bytebuf_read_remaining(&buf) == 15); + + sc_bytebuf_skip(&buf, 3); + assert(sc_bytebuf_read_remaining(&buf) == 12); + + sc_bytebuf_read(&buf, data, 12); + data[12] = '\0'; + assert(!strcmp((char *) data, "hello world!")); + assert(sc_bytebuf_read_remaining(&buf) == 0); + + sc_bytebuf_destroy(&buf); +} + int main(int argc, char *argv[]) { (void) argc; (void) argv; @@ -105,6 +148,7 @@ int main(int argc, char *argv[]) { test_bytebuf_simple(); test_bytebuf_boundaries(); test_bytebuf_overwrite(); + test_bytebuf_two_steps_write(); return 0; }