perf(moq-mux): SIMD-accelerate MPEG-TS sync byte resync#1695
Conversation
The TS demuxer scanned forward one byte at a time to find the next 0x47 sync byte after a misalignment. Replace the byte-by-byte loop with memchr::memchr, which is SIMD-accelerated and already a dependency. The end state is identical (jump to the next sync byte, or drop the buffer if none remains), so resync semantics are unchanged.
|
Warning Review limit reached
More reviews will be available in 3 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR changes Import::decode's resynchronization: when the byte at the current offset is not 0x47, it now calls memchr to locate the next 0x47 in the remaining scratch buffer. If memchr finds no match, the offset is set to scratch.len() so the packet loop exits. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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.
🧹 Nitpick comments (1)
rs/moq-mux/src/container/ts/import.rs (1)
145-148: ⚡ Quick winUse
if letformemchrresult unwrapping consistency.This
matchis only unwrappingOptionand setting fallback state; converting toif letaligns with the repo Rust style rule.Suggested refactor
- off = match memchr::memchr(0x47, &self.scratch[off..]) { - Some(rel) => off + rel, - None => self.scratch.len(), - }; + if let Some(rel) = memchr::memchr(0x47, &self.scratch[off..]) { + off += rel; + } else { + off = self.scratch.len(); + }As per coding guidelines, "
**/*.rs: Preferif let/let ... elseover an unwrappingmatchwhen the only job is to unwrap."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rs/moq-mux/src/container/ts/import.rs` around lines 145 - 148, Replace the match that unwraps memchr::memchr with an if let to match repo style: call memchr::memchr(0x47, &self.scratch[off..]) and use `if let Some(rel) = ... { off = off + rel; } else { off = self.scratch.len(); }`, preserving the exact semantics for the `off` update; reference the `off` variable, `self.scratch`, and the memchr::memchr call in import.rs.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@rs/moq-mux/src/container/ts/import.rs`:
- Around line 145-148: Replace the match that unwraps memchr::memchr with an if
let to match repo style: call memchr::memchr(0x47, &self.scratch[off..]) and use
`if let Some(rel) = ... { off = off + rel; } else { off = self.scratch.len();
}`, preserving the exact semantics for the `off` update; reference the `off`
variable, `self.scratch`, and the memchr::memchr call in import.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 935f8402-4feb-4659-ba18-da4864e9dc55
📒 Files selected for processing (1)
rs/moq-mux/src/container/ts/import.rs
Match repo Rust convention preferring if let over an unwrapping match.
Summary
You asked me to look for code that could be SIMD-optimized, but only where it's a simple change. After surveying the byte-scanning hot loops across
rs/andjs/, there was exactly one clean win.The MPEG-TS demuxer (
rs/moq-mux/src/container/ts/import.rs) resyncs after a lost sync byte by scanning forward one byte at a time looking for the next0x47:This now jumps straight to the next sync byte with
memchr::memchr, which is SIMD-accelerated (SSE2/AVX2/NEON) and was already a dependency of the crate:The end state is identical to the old loop (advance to the next sync byte, or drop the whole buffer when none remains), so resync semantics are unchanged. This matters most when a live capture joins mid-stream or a stream loses sync, where the old path crawled byte-by-byte over the buffer.
What I deliberately left alone
rs/moq-mux/src/codec/annexb.rs) already usesmemchr::memmem::find, which is SIMD-accelerated. No change needed.Test plan
cargo test -p moq-mux(all 36container::tstests pass, includingimport_resyncs_after_byte_misalignment,stays_desynced_until_next_pusi,tei_continuation_drops_partial_and_resyncs)cargo clippy -p moq-muxclean(Written by Claude)
Generated by Claude Code