Skip to content

chore(bycorf): adopt SPDK and submission status optimizations - #419

Merged
thweetkomputer merged 1 commit into
eloqdata:mainfrom
thweetkomputer:chore/bycorf-submission-optimizations
Nov 3, 2026
Merged

thweetkomputer merged 1 commit into
eloqdata:mainfrom
thweetkomputer:chore/bycorf-submission-optimizations

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Context

Adopt the two merged Bycorf submission optimizations:

Behavior before and after

Lavik uses the same submission APIs. SPDK retains sector/range validation, and io_uring retains initialization checks, FIFO admission and completion behavior. Both changes reduce generated submission-path work without changing data-buffer or registration lifetimes.

Implementation

Update only the Bycorf submodule pin, from ac35bac748034f33a49a34d23b3b305287417077 to upstream 7c377436989d9219e2aef4b45638d1d53fd4673b, which contains both merged PRs.

Design decisions and alternatives

Use the merged upstream commit so the two independently reviewed optimizations are consumed together. No Lavik application adaptation is needed for this update.

Documentation and comments

The dependency includes the implementation comments and SPDK boundary tests. No architecture claims change: backend selection, ownership, queue ordering and completion lifetimes remain unchanged.

Test plan

Run from the Lavik repository root:

cmake --build build --target lavik --parallel 6
cmake --build bycorf/build-testcases-bypass --parallel 4
ctest --test-dir bycorf/build-testcases-bypass --output-on-failure
pre-commit run
git diff --cached --check

Lavik's GCC Release build and LTO link passed with LAVIK_ENABLE_WERROR=ON. The merged Bycorf revision passed all 130 bypass-capable tests. Its existing test configuration uses CMAKE_BUILD_TYPE=Debug, BYCORF_BUILD_EXAMPLES=OFF and BYCORF_KERNEL_BYPASS=ON. Whitespace checks passed; pre-commit hooks had no applicable files for the submodule-only change.

Risk assessment

Regression coverage includes io_uring admission/cancellation/shutdown and SPDK sector arithmetic, backpressure and completion ownership. Physical NIC/NVMe throughput and latency were not measured; upstream optimization evidence is based on static code-generation comparisons.

Rollback plan

Revert this commit to restore the previous Bycorf pin.

Reviewer guide

Review the implementation and assembly evidence in Bycorf #30 and #31, then verify the single gitlink update points at the merged upstream revision.

Follow-up work

None required for this dependency update.

Summary by CodeRabbit

  • Chores
    • Updated an underlying component reference. No changes to user-facing features or behavior are indicated in this update.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The bycorf subproject reference advances from commit ac35bac748034f33a49a34d23b3b305287417077 to 7c377436989d9219e2aef4b45638d1d53fd4673b.

Changes

Bycorf reference update

Layer / File(s) Summary
Update bycorf reference
bycorf
The subproject reference changes to commit 7c377436989d9219e2aef4b45638d1d53fd4673b.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Possibly related PRs

  • eloqdata/lavik#418: Pins the preceding Bycorf commit that this PR replaces.
  • eloqdata/lavik#417: Adopts Bycorf's StorageBackend facade, which the updated subproject revision preserves.

Suggested labels: performance-watch

Suggested reviewers: liunyl

Merge Risk: Minimal · up to e4cd8

No actionable merge-blocking issue is identified for this dependency update; it is mergeable subject to normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title names the Bycorf submodule and the two merged optimizations (SPDK sector arithmetic and submission Status construction). The raw summary shows only a one-line submodule pin change, which mat…
Description check Passed The description follows the repository template. It covers Context, Behavior before and after, Implementation, Design decisions, Documentation, Test plan, Risk assessment, Rollback plan, Reviewer guid…
Docstring Coverage Passed 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 Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the Bycorf submodule and the two merged optimizations (SPDK sector arithmetic and submission Status construction). The raw summary shows only a one-line submodule pin change, which matches the title's scope. The title uses the conventional commit prefix and is 61 characters long, which fits the length guideline.

Full details: Description check

Explanation

The description follows the repository template. It covers Context, Behavior before and after, Implementation, Design decisions, Documentation, Test plan, Risk assessment, Rollback plan, Reviewer guide, and Follow-up work. It gives specific commit hashes, linked upstream PRs, the exact build and test commands, and the test configuration. The template's architecture documentation note is addressed in the Documentation section. The Test plan lists commands and results, and the Risk assessment names the coverage gaps. The description is complete and specific.

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 0 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

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

A rabbit checks the commit line,
One new pointer, neat and fine.
Bycorf hops to newer ground,
A quiet change with no loud sound.
Then off through clover, ears held high.

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: 8d4a7b08-ad84-4d7d-8fec-03f5184846e6
Commits

Reviewing files that changed from the base of the PR and between 00629bb and e4cd876.

Files selected for processing: 1
  • bycorf

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

@thweetkomputer
thweetkomputer merged commit 8d54ce0 into eloqdata:main Nov 3, 2026
20 of 22 checks passed
@thweetkomputer
thweetkomputer deleted the chore/bycorf-submission-optimizations branch November 3, 2026 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant