docs: benchmark repro#91
Merged
Merged
Conversation
The INFO reply was sent via the generic send_reply(), which stamps STATUS_SUCCESS as the frame type byte. The client decoder therefore routed INFO through the VALUE path and printed the metrics block quoted with a trailing space, never reaching the dedicated CLIENT_RESPONSE_INFO plain-print branch. Add a dedicated send_info_reply() that emits the CMD_INFO type byte (mirroring send_keys_reply) and switch handle_info_command to use it, so INFO decodes to CLIENT_RESPONSE_INFO and renders as a plain multi-line block. The existing test_info_response_uses_declared_payload_length now reflects the real server wire format instead of an unreachable branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
handle_keys_command formatted every entry as "N) key\n", so the payload ended with a newline after the final key. Combined with the client's print_plain_payload_line (which appends its own newline), this rendered a blank trailing line. Treat newlines as separators between entries and send used - 1 bytes to omit the trailing newline. Safe because the else branch only runs when at least one entry was written. Matches the payload shape already asserted by test_keys_response_prints_list_payload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add FKVS_ENABLE_JEMALLOC CMake option (default OFF) that links fkvs-server against jemalloc; linked unprefixed so it transparently overrides malloc/free with no call-site changes - Add get_allocator_name() reporting the jemalloc version via mallctl, or "system (libc malloc)" otherwise - Log the active allocator at server startup and report it in INFO as mem_allocator (buffer widened to fit the version string) - Forward a JEMALLOC variable through Makefile.fkvs (JEMALLOC=ON) - Install jemalloc in the Dockerfile behind an ENABLE_JEMALLOC build arg - Document building with jemalloc on macOS and Linux (docs/jemalloc.md, README) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hashtable was a fixed 8092-bucket separate-chaining table with no resize path, so distinct-key workloads (e.g. benchmark -r) piled entries into ever-longer chains. set/get/delete walk the chain, making each op O(load factor) and N inserts O(N^2/buckets) — throughput collapsed as the keyspace grew. Add Redis dict-style incremental resizing: - hashtable_t now holds two sub-tables plus a rehash cursor; a resize doubles capacity at load factor 1.0 and migrates one bucket per insert, so no single op stalls on a full rehash - new keys insert into the new table during a resize (old table only drains, guaranteeing termination); lookups/deletes consult both tables - migration only relinks existing nodes, never copies keys/values - sizes are powers of two, so bucket index is a mask, not a division; TABLE_SIZE 8092 -> 8192 - KEYS and expire_sweep now scan both sub-tables so no key is missed mid-resize With this, set/get return to amortized O(1) and -r throughput stays flat as the keyspace grows instead of degrading. Also add a stress test covering inserts, mid-resize reads, in-place updates and deletes across many resize generations, and add a get_allocator_name stub to test_integration (it does not link memory.c). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The -r flag generates a unique key per insertion (a per-process random prefix plus a monotonic counter), not uniformly random keys. Update the usage string and benchmarking guide to say "unique" to match behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each worker generated its key and malloc'd the command buffer inside the send loop, so -r measurements included client-side key generation and a malloc/free per request. Pre-build all command buffers per worker before the start gate: unique-key mode builds one distinct command per request, other modes build a single reused buffer. The timed loop now only calls send() on a pre-built buffer. This makes -r measure server throughput more directly (its result is now in line with the fixed-key path on the client side, differing only by the server's insert-vs-update work). Pre-build memory scales with the request count, which is the same order as the keyspace the server stores. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make FKVS_ENABLE_JEMALLOC default ON on Linux (off elsewhere), mirroring the FKVS_ENABLE_IO_URING pattern. When jemalloc is enabled but the library is not found, the build now warns and falls back to the system allocator instead of failing — so a Linux build without libjemalloc still succeeds. - CMakeLists: platform-aware option default; missing jemalloc is a status message, not a fatal error - Dockerfile: ENABLE_JEMALLOC build arg now defaults to ON - Makefile.fkvs: only pass -DFKVS_ENABLE_JEMALLOC when JEMALLOC is set explicitly, so the plain shortcut inherits the platform default - docs/README: document jemalloc-by-default-on-Linux and how to opt out Verified via Docker: the default image links libjemalloc and reports the jemalloc version at startup; --build-arg ENABLE_JEMALLOC=OFF falls back to the system allocator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document throughput across pipeline depth, event-loop backend (epoll/io_uring) and allocator (jemalloc/system), with the machine configuration and methodology so the numbers are interpretable. Includes both the fixed-key (update-in-place) sweep and the -r allocation-heavy case, plus notes that the Linux figures are from a virtualized VM over loopback TCP and are relative, not absolute. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three single-core optimizations that lift peak SET throughput from ~2.4M to ~2.9M req/s sustained (peaks ~3.1-3.5M) on the benchmark VM at -P 128: - handle_set_command allocated, copied and freed a value buffer on every request solely to feed a verbose-only debug print. Print straight from the request buffer with %.*s and drop the allocation. - set_value did calloc + malloc + 2x free per update. Add a fast path that overwrites an existing value of the same length in place, with no allocation, which is the common case for repeated SETs. - try_process_frames memmove'd the remaining read buffer after every frame (O(bytes x frames) for a deep pipeline). Parse forward with a cursor and compact once at the end. Verified: full test suite passes; benchmark reports 0 failures for both fixed-key and -r workloads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-run the sweep after the SET hot-path optimizations: peak single-core throughput rises to ~2.9M req/s at -P 128 (peaks ~3.1-3.5M). With per-op allocation largely removed, jemalloc and the system allocator now perform within noise, so the reading notes are updated accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 'Merge branch main' conflict resolution duplicated set_value and scrambled its body (a truncated fast-path copy returning NULL from a bool, plus a second definition with undeclared variables and reordered insert logic), which failed every CI build with 'redefinition of set_value'. Restore the single, correct function: validate, advance any resize, same-length in-place fast path, then allocate-and-insert. Build and full test suite pass; hashtable tests are clean under ASan/UBSan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.