You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
chore(bycorf): adopt SPDK and submission status optimizations - #419
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.
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.
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: 5
Check name
Status
Explanation
Title check
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
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
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: 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.
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.
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
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.
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
ac35bac748034f33a49a34d23b3b305287417077to upstream7c377436989d9219e2aef4b45638d1d53fd4673b, 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:
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 usesCMAKE_BUILD_TYPE=Debug,BYCORF_BUILD_EXAMPLES=OFFandBYCORF_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