[thweetkomputer]

Context

Adopt the merged Bycorf fixed-buffer submission optimization from eloqdata/bycorf#29. Previously, forwarding a fixed-buffer request could materialize several copies of the descriptor on the stack, including on the SPDK path.

Behavior before and after

Worker, StorageBackend and the concrete backend now borrow the FixedBuffer descriptor during synchronous submission. The backend snapshots queued request parameters before returning; the actual data buffer and registration still live through completion. Lavik's existing submission calls remain compatible.

Implementation

Advance the Bycorf submodule from 4b10a34e8eff7d991018454a682019773e3a112e to the upstream merge commit ac35bac748034f33a49a34d23b3b305287417077. Also default CompactAccessOptions::stream_inspect_ to std::nullopt, matching the adjacent optional fields. Partial aggregate initialization on current main otherwise fails the GCC build with -Werror=missing-field-initializers; this preserves the existing empty optional value.

Design decisions and alternatives

Use the merged upstream revision. Its source tree is identical to the tested Bycorf PR #29 head; awaitables continue to own descriptors and queued I/O continues to own its prepared parameters.

Documentation and comments

No architecture claim changes: backend selection, shared-ring ownership, and completion lifetimes remain the same. The dependency contains the descriptor-borrowing API comments and regression tests.

Test plan

From the Lavik repository root:

cmake --build build --target lavik --parallel 6
git diff --check
pre-commit run --files include/lavik/storage/engine.h

Lavik's GCC Release build and LTO link passed with LAVIK_ENABLE_WERROR=ON. Formatting and whitespace checks passed.

The identical upstream Bycorf source tree previously passed 96 normal tests and 123 bypass-capable tests, including descriptor reuse/reset while fixed reads and writes wait in io_uring or SPDK queues. These unchanged dependency tests were not rerun for the pin update.

Risk assessment

The fixed-buffer submission signatures change at the binary interface, so dependents must be rebuilt. The Lavik executable was rebuilt against the updated dependency. Upstream instruction/stack reductions are static code-generation evidence; hardware throughput and latency were not measured.

Rollback plan

Revert this commit to restore the previous Bycorf pin.

Reviewer guide

Review the implementation, ownership regression tests and assembly evidence in Bycorf #29, then verify the merged pin and the one-line optional default initializer.

Follow-up work

None required for this dependency update.

Summary by CodeRabbit

• Chores
  • Updated project maintenance details. No user-facing behavior changes are included in this update.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The bycorf subproject reference changes to a new commit. CompactAccessOptions::stream_inspect_ now explicitly initializes to std::nullopt.

Changes

Bycorf subproject reference

Layer / File(s)	Summary
Update subproject reference bycorf	The recorded subproject commit changes from 4b10a34e8eff7d991018454a682019773e3a112e to ac35bac748034f33a49a34d23b3b305287417077.

Stream inspection option default

Layer / File(s)	Summary
Initialize stream inspection option include/lavik/storage/engine.h	CompactAccessOptions::stream_inspect_ now explicitly initializes to std::nullopt.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Refactor

Possibly related PRs

• eloqdata/lavik#417: Adopts Bycorf's StorageBackend facade, which is related to the Bycorf subproject reference update.
• eloqdata/lavik#377: Adds caller-owned FixedBuffer workspaces for storage I/O, related to the fixed-buffer submission work described in the PR context.
• eloqdata/lavik#384: Routes storage reads through fixed-buffer arena ranges, also related to fixed-buffer storage I/O.

Suggested labels: performance-watch

Suggested reviewers: liunyl

Merge Risk: Minimal · up to 1c9cc

No actionable merge-blocking issue is identified. The dependency update changes the binary interface, so dependents should be rebuilt as noted in the PR.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly identifies the Bycorf fixed-buffer submission optimization adopted by this change.
Description check		The description covers all template sections, explains the changes and design decisions, reports tests and risks, and provides rollback and reviewer guidance.
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)

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.

ast-grep (0.51.1)

ast-grep could not parse rule config: /ast-grep-rules/ast-grep-essentials/rules/go/security/zip-slip-filepath-join-archive-entry-go.yml

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 checks the options bright
std::nullopt sits just right
A new Bycorf commit hops along
The storage paths hum a song
Soft paws close the review tonight}

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 299ffb10-0b01-4722-aab3-58744a0892d5

Commits

Reviewing files that changed from the base of the PR and between 8115c94 and 1c9ccd6.

Files selected for processing: 2

• bycorf
• include/lavik/storage/engine.h

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