Repository navigation
Conversation
The global ENTRY-array append opened the tail array read-only for its capacity and then mutably to write the slot; it now opens it mutably once. The regular DATA-array link reopened the tail after the chain walk had already read its capacity; it now keeps that value. Mutable opens run the same type, size and bounds checks, and publication order is unchanged. Regular-format writes execute 3.9-5.1% fewer instructions; output is byte-identical. Completes SOW-0137.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | -13 |
| Duplication | -3 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
✅ No blocking issues noticed
No blocking implementation findings were identified. The changed paths lack automated tests covering reuse, growth, validation, and reader interoperability; these should be added to protect the refactoring.
About this PR
- Please add automated tests covering the changed ENTRY/DATA append paths, growth and linking behavior, malformed-input validation, and reader interoperability.
Test suggestions
- Append an ENTRY entry when the tail has capacity, verifying the slot and header updates.
- Append a regular DATA entry when the tail has capacity, using the capacity discovered during traversal.
- Exercise ENTRY and DATA array growth paths and verify correct linking and tail metadata.
- Reject malformed, wrong-type, undersized, or out-of-bounds arrays with the same validation behavior.
- Verify byte-identical output and interoperability with journal readers.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Append an ENTRY entry when the tail has capacity, verifying the slot and header updates.
2. Append a regular DATA entry when the tail has capacity, using the capacity discovered during traversal.
3. Exercise ENTRY and DATA array growth paths and verify correct linking and tail metadata.
4. Reject malformed, wrong-type, undersized, or out-of-bounds arrays with the same validation behavior.
5. Verify byte-identical output and interoperability with journal readers.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Architecture diagram
sequenceDiagram
participant W as Writer
participant JF as JournalFile
participant OA as OffsetArray
Note over W,OA: ENTRY Array Append Flow (C1)
W->>JF: entry_array_tail_entries()
JF-->>W: tail_entries count
W->>JF: offset_array_mut(tail_offset)
JF->>OA: validate & open mutable
OA-->>JF: capacity
JF-->>W: tail_guard with capacity
alt Room in tail array
W->>OA: set(tail_entries, entry_offset)
W->>JF: journal_header_mut()
JF->>JF: update tail_entry_array_offset & n_entries
else Tail array full
W->>JF: drop(tail_guard)
W->>JF: grow_entry_array_tail()
end
Note over W,OA: Regular DATA Array Link Flow (C2)
W->>JF: append_data_entry_array_link()
W->>JF: walk array chain from head
JF->>OA: offset_array_ref (per array in chain)
OA-->>JF: next_offset & capacity
JF-->>W: tail_offset & tail_capacity (from walk)
alt Entries fit in tail
W->>JF: offset_array_mut(tail_offset)
JF->>OA: validate & open mutable
W->>OA: set slot with entry link
else Tail full
W->>JF: allocate & grow new array
JF-->>W: new tail offset
W->>JF: offset_array_mut(new_tail)
W->>OA: set slot 0
end
Note over W,OA: Validation Remains (all paths)
JF->>JF: type/size/arena checks before mutation
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
# Conflicts: # .agents/sow/SOW-status.md
Removes the reopen of each freshly allocated entry array to write slot 0. Mixed-cardinality workloads allocate an array per new DATA object, so this saves about 0.4% of writer instructions there; output bytes are unchanged.
# Conflicts: # .agents/sow/SOW-status.md
# Conflicts: # .agents/sow/SOW-status.md
# Conflicts: # .agents/sow/SOW-status.md
Stacked on #16. Merge order: #13, #14, #15, #16, then this. Completes SOW-0137 (
.agents/sow/done/SOW-0137-20260726-rust-writer-residual-array-open-performance.md).Decisions for review (all agent-chosen; internal only, no byte or API change)
Mutable opens run the same type, size and bounds checks as read-only ones. Publication order is unchanged.
Measurement
Callgrind instructions at 20,000 rows, before → C1+C2 → C1+C2+C3:
Release timing, baseline vs C1+C2+C3, regular format, 20 ABBA pairs per workload at host load 36-41: mixed +3.7% median (faster in 19/20 pairs), netflow +12.1% (19/20). The host was loaded, so timing is secondary evidence; an earlier -1.4% mixed run is not reproduced but remains unexplained.
Validation
journalctl --verifypasses all four outputs.cargo test --workspacepasses.Summary by cubic
Removes redundant offset-array opens in the Rust writer so regular-format writes execute 3.9-5.1% fewer instructions with byte-identical output. Completes SOW-0137.
journalctl --verifypasses, and the live reader matrix passes 8/8.Written for commit 9dfa6e1. Summary will update on new commits.