perf: reduce allocation and copying in compact point writes

[thweetkomputer]

Context

Updating one element of an approximately 8 KiB compact collection can allocate strings for unchanged elements and rebuild a complete encoded image. Compact String range writes also hold worker store state while reading and preparing their replacement. These costs contribute to the small-write gap investigated in the large-object benchmark report.

Behavior before and after

• List end pushes/pops splice encoded intervals; only popped reply values need owned strings.
• Set SADD/SREM use the checked Hash splice planner, including duplicate operands and deletion of the final member.
• Single-member ZADD/ZINCRBY/ZREM scan and splice the compact encoding without copying every member or sorting the entire collection.
• Existing compact String writes release worker store state during the old-value read and private preparation, retain key intent/database admission, and revalidate before publication.
• Compact Hash/Set/List/Sorted Set/String edits can encode directly into staging instead of allocating a complete intermediate after-image.

Implementation

CompactPayload owns framing and borrows unchanged encoded intervals and command operands. The existing InlineRecordPayload writer now accepts bounded compact records and consumes the plan after its final suspension, before publication and checksum handling. A checked borrowed List/Sorted Set reader validates complete framing without per-element ownership.

Read leases remain alive through append waits. Remote leases and local read-pool exhaustion use owned output so queued writers do not retain the last read slot needed by another operation. Existing TTL, WATCH, transaction, replication, promotion and durable-format machinery remains the publication boundary.

Design decisions and alternatives

Use byte-level edit plans for common bounded point writes and keep owned builders for promotion and other operations. This avoids introducing a mutable hot-object cache or another durable representation. Transactional String preparation retains its existing state-lock boundary.

Documentation and comments

Updated docs/architecture/09-grouped-collections.md for borrowed after-image ownership and compact String preparation. Added API/lifetime, publication-failure and read-pool fallback comments near the implementation.

Test plan

Validation ran on Linux ARM64 with Clang 18 Debug and GCC 13 Release (LAVIK_ENABLE_TRACE=ON). The existing local Bycorf checkout was a37ed7d19885275472b707461dc2eddf4dccfbcd; its update is excluded from this PR, which retains the pinned gitlink 86d686724ddd1c5617fb1b62f8131e05571e8423. CI must also validate the pinned dependency.

Final checks passed:

cmake --build build_ci --target lavik lavik_unit_tests lavik_compact_collection_write_e2e_test --parallel 4
cmake --build build_release_trace --target lavik --parallel 4

build_ci/lavik_unit_tests --gtest_filter='CompactHashEdit.*:OrderedCompactCodecTest.*:RecordPayloadCursorTest.*-OrderedCompactCodecTest.LargeListRoundTripsBeyondRecordLimit:OrderedCompactCodecTest.LargeSortedSetRoundTripsBeyondRecordLimit'
build_ci/lavik_compact_collection_write_e2e_test build_ci/lavik --gtest_filter='CompactStringWriteE2e.*:CompactPointWriteE2e.*:SmallTypes/*'
LAVIK_BIN=$PWD/build_ci/lavik LAVIK_PORT=17391 tests/valkey/run-lavik --single unit/type/string --single unit/type/list --single unit/type/set --single unit/type/zset --single unit/type/hash

build_ci/lavik_multi_exec_e2e_test build_ci/lavik
build_ci/lavik_grouped_hash_write_e2e_test build_ci/lavik --gtest_filter='CompactHashWriteE2e.*:GroupedHashWriteE2e.ColdCompactWritesReleaseStateButKeepTheKeyLocked'
build_ci/lavik_list_e2e_test build_ci/lavik --gtest_filter=CollectionE2eTest.SortedSetGeoAndStreamCommandsRecover
build_ci/lavik_grouped_ordered_write_e2e_test build_ci/lavik --gtest_filter=GroupedDemotionE2e.StringListSetSortedSetGeoAndStreamRecoverCompact:GroupedStringWriteE2e.InlineEncodingFailureKeepsPreviousValueAndWriter:GroupedStringWriteE2e.SegmentNoOpsPreserveWatchAndTtl
build_ci/lavik_compact_collection_write_e2e_test build_release_trace/lavik --gtest_filter=CompactPointWriteE2e.MixedEightKiBEditsMatchModelsAndRecover

pre-commit run
git diff --cached --check

The focused unit run passed 22 tests; the compact E2E run passed 13 tests; all five Valkey type suites passed. New coverage includes 8 KiB mixed edits and recovery, SETRANGE concurrency, ordinary and EXEC encoding failures preserving old values/WATCH, single-read-slot/indirect-key fallback, malformed/truncated framing, and randomized Set deletion plans. Additional legacy memory-limit and transaction rollback cases passed. A startup timeout and a timing-sensitive blocking assertion seen during concurrent builds passed when rerun after build contention ended.

The complete repository test suite and comparable QPS benchmarks have not been run.

Risk assessment

The main correctness risk is borrowed-source lifetime across append suspension, especially under read-pool pressure. The encoder remains private until successful consumption, and fault-injection/recovery tests cover failed publication. The stored encoding and promotion thresholds remain compatible.

Rollback plan

Revert this change; no data migration is required.

Reviewer guide

Start with compact_payload.h and the borrowed readers, then the type-specific planners in hash_codec.cpp, list_tree.cpp, and grouped/zset.cpp. Review the String lock boundary and compact_api.cpp/write.cpp lifetime and publication changes together. The compact E2E tests exercise the shared failure boundary.

Follow-up work

Rerun the comparable 8 KiB workload and collect CPU/allocation profiles to quantify QPS and allocation improvements. This PR makes no measured throughput claim.

Summary by CodeRabbit

• Performance

  • Small edits to hashes, sets, lists, sorted sets, and strings can be applied without rebuilding the entire value, improving write efficiency.
  • Some string updates can proceed while other writes continue.

• Reliability

  • Improved handling of compact collection writes during recovery and low-buffer conditions.
  • Failed writes preserve existing values and transaction watch state.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 491aa144-67cc-43e8-866e-8dc3b9a3bc19

📥 Commits

Reviewing files that changed from the base of the PR and between 684894d and 48ca16c.

📒 Files selected for processing: 18

• docs/architecture/09-grouped-collections.md
• include/lavik/storage/detail/compact_payload.h
• include/lavik/storage/detail/hash_codec.h
• include/lavik/storage/detail/ordered_compact_codec.h
• include/lavik/storage/engine.h
• src/storage/engine/compact_api.cpp
• src/storage/engine/grouped/string.cpp
• src/storage/engine/grouped/zset.cpp
• src/storage/engine/hash_codec.cpp
• src/storage/engine/hash_tree.cpp
• src/storage/engine/impl.h
• src/storage/engine/list_tree.cpp
• src/storage/engine/ordered_compact_codec.cpp
• src/storage/engine/write.cpp
• tests/compact_collection_write_e2e_test.cpp
• tests/grouped/hash_test.cpp
• tests/grouped/write_e2e_support.h
• tests/ordered_compact_codec_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The change adds bounded compact-edit planning and borrowed inline payload encoding for collection and String writes. It extends the append pipeline to accept compact encoders, adds compact Hash, List, and Sorted Set edit paths, changes ordinary compact String preparation, and adds codec and write-path tests.

Changes

Compact Write Preparation

Layer / File(s)	Summary
Inline payload and append pipeline include/lavik/storage/detail/compact_payload.h, include/lavik/storage/engine.h, src/storage/engine/impl.h, src/storage/engine/write.cpp	Adds a payload type that combines borrowed views and owned framing bytes. The append and record-writing paths accept compact inline encoders and validate their size and write conditions.
Compact Hash planning and writes include/lavik/storage/detail/hash_codec.h, src/storage/engine/hash_codec.cpp, src/storage/engine/hash_tree.cpp, tests/grouped/hash_test.cpp	Adds bounded Hash set and delete plans, connects eligible plans to inline writes, and tests deletion counts, output, and malformed input.
List, Sorted Set, and String preparation include/lavik/storage/detail/ordered_compact_codec.h, src/storage/engine/ordered_compact_codec.cpp, src/storage/engine/list_tree.cpp, src/storage/engine/grouped/zset.cpp, src/storage/engine/grouped/string.cpp, src/storage/engine/compact_api.cpp, tests/ordered_compact_codec_test.cpp, tests/compact_collection_write_e2e_test.cpp, tests/grouped/write_e2e_support.h, docs/architecture/09-grouped-collections.md	Adds checked borrowed ordered-entry reading and compact List and Sorted Set edit paths. The String path can prepare eligible existing values outside the state lock and validates its snapshot after reacquiring it. Tests cover encoding, failure handling, recovery, and limited read-buffer availability. The architecture documentation describes payload lifetimes and fallback paths.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant HSET
  participant ExecuteHashLikeLocked
  participant PlanCompactHashEdit
  participant AppendLocked
  participant WriteRecordLocked
  HSET->>ExecuteHashLikeLocked: submit Hash mutation
  ExecuteHashLikeLocked->>PlanCompactHashEdit: plan bounded edit
  PlanCompactHashEdit-->>ExecuteHashLikeLocked: return changed payload plan
  ExecuteHashLikeLocked->>AppendLocked: pass inline payload
  AppendLocked->>WriteRecordLocked: pass compact encoder

Loading

Suggested reviewers: liunyl

Merge Risk: ⚪ Minimal · up to 48ca1

This change reduces copying in compact List, Set, Sorted Set, Hash, and String writes without changing durable formats. The specific risks examined did not hold up: string size grouping, deleting the last collection member, and payload lifetimes all behave correctly. Normal CI should still confirm the build before merge.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 17 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly summarizes the primary change: reducing allocation and copying in compact point writes.
Description check		The description covers all required template sections. It explains the context, behavior changes, implementation, design decisions, documentation updates, exact test commands and results, risks, rollb…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description covers all required template sections. It explains the context, behavior changes, implementation, design decisions, documentation updates, exact test commands and results, risks, rollback, reviewer guidance, and follow-up work.

Full details: Docstring Coverage

Explanation

Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 17 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Cppcheck (2.22.0)

src/storage/engine/compact_api.cpp

Cppcheck timed out; analysis of this file is incomplete

src/storage/engine/grouped/string.cpp

Cppcheck timed out; analysis of this file is incomplete

src/storage/engine/hash_tree.cpp

Cppcheck timed out; analysis of this file is incomplete

• 1 others

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit packs the bytes with care,
Borrowed leaves and owned bits there.
Hashes, lists, and scores align,
Strings wait, then pass the line.
The record takes the payload through,
And wakes to find its contents true.}

_{Comment @coderabbitai help to get the list of available commands.}