fix(decoding): keep the scalar sequence decoder's debug frame off a kernel stack

[myers]

Summary

• A debug (-O0) build of the scalar sequence decoder no longer needs ~145 KiB of stack for one frame. Debug kernels run with small fixed task stacks too, and on main the scalar monolith's own frame is larger than a 96 KiB thread.
• Both causes are shapes upstream zstd already avoids: a const lookup table copied at every use, and an executor that is forced inline even in a debug build.
• Optimized builds keep the forced inline; the only release-visible change is BIT_MASK becoming a static.

Changes

• cpu_kernel::BIT_MASK is a static instead of a const [u64; 256]. A const is a value, so at -O0 each inlined mask_lower_bits load copied all 2 KiB of it into its own stack slot (43 KiB of the frame). Upstream's BIT_mask is static const unsigned BIT_mask[] (lib/common/bitstream.h).
• execute_one_sequence_pipelined is #[inline(always)] only without debug_assertions, and plain #[inline] with them. The monolith expands it at the three execute_one_body! sites. Upstream marks ZSTD_execSequence HINT_INLINE and drops the forced inline under ZSTD_NO_INLINE (lib/common/compiler.h).
• New test scalar_decode_fits_a_bounded_debug_stack enters the scalar monolith for RingBuffer, FlatBuf and UserSliceBackend on a 96 KiB thread. cargo test builds at -O0, so it runs the configuration a debug kernel links. Mutation check: restoring the const alone, the forced inline alone, or both each aborts it with "has overflowed its stack".

Stack

Own frame of each monomorph of decode_and_execute_sequences_scalar, read from the object file of a debug build for x86_64-unknown-none, panic = "abort", no default features:

backend	main	BIT_MASK static	+ executor not forced
UserSliceBackend	147,912 B	104,648 B	25,736 B
FlatBuf	88,648 B	45,384 B	23,784 B
RingBuffer	83,912 B	40,648 B	23,784 B

The out-of-line executor adds 26,376 B (UserSliceBackend) when called.

Performance

profile_decode against main, separately built binaries, min of seven interleaved runs, aarch64 (Apple silicon):

input	main	this PR
z000033.zst, 300 iters	0.700 s	0.700 s
z000091.zst, 300 iters	0.229 s	0.229 s
z000006.zst, 200k iters	3.704 s	3.713 s

All inside the build-to-build band; no change established in either direction. The x86_64 scalar tier was not timed, and not measured against libzstd.

Testing

fmt; clippy with -D warnings (all targets; library without default features); the crate tests on macOS aarch64.

Summary by CodeRabbit

• Bug Fixes
  • Improved decoding reliability when running on threads with limited stack space, including in debug builds.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: Repository: structured-world/structured-zstd/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 5d27966e-2306-4482-a789-4017af9b3e0a

---

📥 Commits

Reviewing files that changed from the base of the PR and between dd33084 and b74281c.

---

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

Walkthrough

The changes adjust BIT_MASK storage and sequence-executor inlining behavior. A std-only regression test runs scalar sequence decoding on a bounded stack for RingBuffer, FlatBuf, and UserSliceBackend.

Changes

Debug stack usage

Layer / File(s)	Summary
Stack usage adjustments zstd/src/cpu_kernel.rs, zstd/src/decoding/sequence_section_decoder.rs	BIT_MASK changes from const to static. The sequence executor uses forced inlining in optimized builds and ordinary inlining in debug builds.
Bounded-stack regression test zstd/src/decoding/sequence_section_decoder/tests.rs	A std-only test runs scalar decoding on a 96 KiB thread stack for RingBuffer, FlatBuf, and UserSliceBackend.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: polaz

---

---

---

---

Merge Risk: ⚪ Minimal · up to dd330

The debug-stack regression test runs in the documented test configuration; no actionable issue remains before merge.

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: reducing the scalar sequence decoder's debug stack frame on kernel stacks. It matches the source changes and PR objectives.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files.
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.

---

---

---

---

---

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[structured-world-cla]

Thank you for the pull request. Before it can be merged, every author of its commits signs the Contributor License Agreement once. You keep the copyright in your contribution; the agreement is a license to the maintainer.

Sign here: https://sw.foundation/cla?pr=https%3A%2F%2Fgithub.com%2Fstructured-world%2Fstructured-zstd%2Fpull%2F551

Not signed yet: @myers

The status updates by itself once everyone has signed.

[coderabbitai]

🔇 Additional comments (3)

zstd/src/cpu_kernel.rs (1)

505-518: LGTM!

zstd/src/decoding/sequence_section_decoder.rs (1)

518-519: LGTM!

zstd/src/decoding/sequence_section_decoder/tests.rs-399-474 (1)

399-474: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Stack-overflow regression test depends on the exact -O0 profile.

The test aborts the whole test process, not only itself, if the stack overflows. This is intended. The test also stays valid only when the test profile keeps opt-level = 0. A profile override with opt-level above 0 would pass the test and hide a regression. Consider a cfg(debug_assertions) gate or a comment that names this dependency. The current doc comment already states that cargo test builds at -O0, so this is optional.

The test code is otherwise correct. std use is allowed in test code by the coding guidelines. The leaked 4 KiB slice is acceptable in a test.

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: structured-world/structured-zstd/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 70be2393-86f2-4429-8697-f72d5684cf31

📥 Commits

Reviewing files that changed from the base of the PR and between f2f1f75 and dd33084.

📒 Files selected for processing (3)

• zstd/src/cpu_kernel.rs
• zstd/src/decoding/sequence_section_decoder.rs
• zstd/src/decoding/sequence_section_decoder/tests.rs

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