Reduce CheetahString to a provenance-safe 24-byte layout - #149
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThe PR replaces the inline length byte with a constrained enum to enable a provenance-safe 24-byte representation. It adds layout and allocation contract checks, Windows 32-bit CI coverage, and documentation for the representation and verification requirements. ChangesCompact
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Reduce CheetahString’s footprint to a provenance-safe 24-byte layout using enum niches, and extend CI/tests/docs/evidence to enforce the new stable layout across 32-bit and 64-bit targets.
Changes:
- Introduce a constrained
InlineLengthenum to enable niche-based 24-byte layouts forCheetahStringandOption<CheetahString>. - Expand layout/allocation evidence (schema v3) and add downstream container slot checks.
- Add 32-bit (i686 Windows MSVC) CI coverage and strengthen tests/docs around the stable layout contract.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/layout_snapshot.rs | Updates layout assertions and snapshots for 24-byte CheetahString and tuple slot footprints. |
| tests/basic.rs | Adds a regression test ensuring Option<CheetahString> round-trips across all storage modes. |
| src/inline.rs | Replaces u8 length with constrained InlineLength enum and adds unit tests for layout/coverage. |
| scripts/verify-allocation-evidence.py | Bumps evidence schema to v3 and validates new downstream slot footprint fields. |
| scripts/tests/test_repository_contracts.py | Adds contract checks preventing pointer-integerization and enforces i686 layout CI command presence. |
| scripts/tests/test_allocation_evidence.py | Updates evidence schema/version and adds a regression test for downstream slot footprint. |
| scripts/bench-all.sh | Updates bench schema IDs and contract JSON to schema v3 + new size/slot fields. |
| scripts/bench-all.ps1 | Same as bench-all.sh for PowerShell environments. |
| benches/shared_backing.rs | Emits schema v3 evidence, including downstream slot footprint metrics. |
| SAFETY.md | Documents the niche-based 24-byte layout and its provenance/pointer-safety story. |
| README.md | Documents the 24-byte sizes and downstream slot savings, linking to the stable layout doc. |
| PERFORMANCE.md | Updates performance contracts to include 24-byte layouts and downstream slot footprints; schema v3 wording. |
| LAYOUT.md | Adds a new stable layout contract document describing the niche strategy and enforced sizes. |
| CHANGELOG.md | Notes the size reduction and provenance-safe niche approach. |
| .github/workflows/ci.yaml | Adds a dedicated i686 Windows job to enforce the 32-bit compact layout. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+35
to
+38
| - Reduced `CheetahString` and `Option<CheetahString>` from 32 to 24 bytes on | ||
| 64-bit targets while preserving the 23-byte inline capacity. The stable | ||
| representation uses constrained Rust enum discriminants as layout niches and | ||
| does not encode pointers as integers. |
Comment on lines
+53
to
+63
| def test_compact_layout_does_not_integerize_or_reconstruct_pointers(self) -> None: | ||
| source = read("src/inline.rs") | ||
| self.assertIn("enum InlineLength", source) | ||
| self.assertEqual(source.count("unsafe {"), 2) | ||
| for forbidden in ( | ||
| "from_raw_parts", | ||
| "transmute", | ||
| "expose_provenance", | ||
| "with_exposed_provenance", | ||
| ): | ||
| self.assertNotIn(forbidden, source) |
Comment on lines
+41
to
+68
| const fn new(len: usize) -> Option<Self> { | ||
| match len { | ||
| 0 => Some(Self::L0), | ||
| 1 => Some(Self::L1), | ||
| 2 => Some(Self::L2), | ||
| 3 => Some(Self::L3), | ||
| 4 => Some(Self::L4), | ||
| 5 => Some(Self::L5), | ||
| 6 => Some(Self::L6), | ||
| 7 => Some(Self::L7), | ||
| 8 => Some(Self::L8), | ||
| 9 => Some(Self::L9), | ||
| 10 => Some(Self::L10), | ||
| 11 => Some(Self::L11), | ||
| 12 => Some(Self::L12), | ||
| 13 => Some(Self::L13), | ||
| 14 => Some(Self::L14), | ||
| 15 => Some(Self::L15), | ||
| 16 => Some(Self::L16), | ||
| 17 => Some(Self::L17), | ||
| 18 => Some(Self::L18), | ||
| 19 => Some(Self::L19), | ||
| 20 => Some(Self::L20), | ||
| 21 => Some(Self::L21), | ||
| 22 => Some(Self::L22), | ||
| 23 => Some(Self::L23), | ||
| _ => None, | ||
| } |
Comment on lines
8
to
13
| ROOT = Path(__file__).resolve().parents[2] | ||
| WORKFLOWS = ROOT / ".github" / "workflows" | ||
|
|
||
|
|
||
| def read(path: str) -> str: | ||
| return (ROOT / path).read_text(encoding="utf-8") |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
OptionCheetahStringandOption<CheetahString>to 24 bytes while preserving the 23-byte SSO boundary&'static strandArc<str>pointers without integer conversion, reconstruction, unions, or manual drop logicCompatibility
cargo-semver-checkspassed 196 checks againstorigin/main; no semver update is requireddocs/ordocs/rocketmq-docare includedEvidence
CheetahString24 bytes,Option<CheetahString>24 bytes,(CheetahString, u64)32 bytesCheetahString24 bytes andOption<CheetahString>24 bytes, executed rather than compile-onlyVerification
Optionstorage modeCloses #145
Summary by CodeRabbit
Performance
CheetahStringandOption<CheetahString>memory usage to 24 bytes while preserving 23-byte inline capacity.Documentation
Tests