Repository navigation
feat(extract): add SIMD-accelerated FASTQ parsing - #180
Merged
Merged
Conversation
nh13
force-pushed
the
feat/nh/simd-fastq-parser
branch
from
March 23, 2026 16:06
02dd74a to
a6f532e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #180 +/- ##
==========================================
+ Coverage 84.29% 85.65% +1.36%
==========================================
Files 128 128
Lines 51978 51964 -14
==========================================
+ Hits 43814 44510 +696
+ Misses 8164 7454 -710 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nh13
force-pushed
the
feat/nh/simd-fastq-parser
branch
from
March 23, 2026 16:11
a6f532e to
3eb3efc
Compare
nh13
marked this pull request as ready for review
March 23, 2026 16:12
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughA new workspace crate 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
crates/fgumi-simd-fastq/benches/fastq_parsing.rs (1)
91-105: Make the real-data benchmark portable.The
/Volumes/...path is workstation-local, so this benchmark silently disappears everywhere else. An env var keeps the same opt-in behavior without baking a machine-specific path into the repo.Possible tweak
- let real_fastq_path = "/Volumes/scratch-00001/work/rebgzf_test.fastq"; - if std::path::Path::new(real_fastq_path).exists() { - let data = std::fs::read(real_fastq_path).unwrap(); + if let Ok(real_fastq_path) = std::env::var("FGUMI_REAL_FASTQ") { + let data = std::fs::read(&real_fastq_path).unwrap(); let data_size = data.len(); group.throughput(Throughput::Bytes(data_size as u64)); group.bench_with_input(BenchmarkId::new("simd", "real_176MB"), &data, |b, data| { b.iter(|| find_record_offsets(data)); }); group.bench_with_input(BenchmarkId::new("memchr", "real_176MB"), &data, |b, data| { b.iter(|| find_record_offsets_memchr(data)); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-simd-fastq/benches/fastq_parsing.rs` around lines 91 - 105, Replace the hardcoded workstation path by reading an opt-in environment variable (e.g. REAL_FASTQ_PATH) and only run the real-data benchmarks when that env var is set and points to an existing file; specifically, replace usage of the literal real_fastq_path with a call to std::env::var("REAL_FASTQ_PATH").ok().and_then(|p| if Path::new(&p).exists() { Some(p) } else { None }), then read the file and call group.throughput and group.bench_with_input as before using find_record_offsets and find_record_offsets_memchr so the benchmark is portable but still opt-in.src/lib/fastq.rs (1)
326-328: Avoid the extra full-copy here.
SimdFastqReaderalready materializesname,sequence, andqualityas ownedVec<u8>values, andfrom_record_with_structure()clones that payload again intoFastqSet/segment buffers. On this path that's a second full copy of every record, which works against the parser swap's performance goal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/fastq.rs` around lines 326 - 328, The code is cloning record.name/sequence/quality into from_record_with_structure(), causing a second full copy; update the call and the from_record_with_structure()/FastqSet constructors to take ownership of the Vec<u8> produced by SimdFastqReader (e.g., accept name: Vec<u8>, sequence: Vec<u8>, quality: Vec<u8> or use mem::take/into_owned on the record) instead of borrowing &record.name etc., so you move the buffers directly from the SimdFastqReader record into the FastqSet/segment without an extra clone; adjust signatures and call sites accordingly (from_record_with_structure, FastqSet fields) to reflect owned Vec<u8> consumption.crates/fgumi-simd-fastq/src/lexer.rs (1)
513-522: Userstestfor these sweep cases.These are parameterized tests in disguise. Splitting them into
rstestcases will make failures much easier to localize without changing coverage.As per coding guidelines,
**/*.rs: Use rstest for parameterized tests.Also applies to: 551-571
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-simd-fastq/src/lexer.rs` around lines 513 - 522, Replace the manual loop in the test function test_simd_newlines_every_position with an rstest parameterized test: add the #[rstest] attribute to the test, change it to accept a parameter pos: usize, and provide pos values covering 0..64 (so each position becomes an individual case); inside the test body keep the same setup using lex_block and lex_block_scalar and the assert_eq!(simd_result, scalar_result, "Mismatch at position {pos}"); do the same refactor for the similar sweep test around lines 551-571 so each pos is a separate rstest case (referencing the same function names lex_block and lex_block_scalar to locate the code).crates/fgumi-simd-fastq/src/parser.rs (1)
190-327: Tests look solid. Consider proptest for fuzz coverage.Good edge cases covered. Per coding guidelines, property-based testing with proptest could catch malformed-input edge cases the SIMD path might encounter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-simd-fastq/src/parser.rs` around lines 190 - 327, Add property-based tests using proptest to fuzz the FASTQ parser paths: add proptest to dev-dependencies and create a new test that generates arbitrary byte buffers and asserts invariants for find_record_offsets and parse_records — e.g., offsets from find_record_offsets are nondecreasing, start with 0, each offset < = buffer.len(), if the final offset == buffer.len() then parse_records yields only complete records whose concatenation matches the original complete-record bytes, and parsed record fields (name, sequence, quality) obey expected separators; target both find_record_offsets and parse_records when writing strategies (including random ASCII/newline distributions and small mutations) to catch SIMD/misalignment issues and boundary-spanning records.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/fgumi-simd-fastq/src/parser.rs`:
- Around line 149-154: The documentation for parse_single_record promises a
panic on malformed input but the code uses debug_assert!, which is skipped in
release builds; replace the debug_assert! in parse_single_record with a regular
assert! (keeping the same condition and message) so the check and its clear
panic message run in all builds, or alternatively update the docstring to state
the check is debug-only—pick one and make the change around the assert in
parse_single_record.
In `@crates/fgumi-simd-fastq/src/reader.rs`:
- Around line 124-128: The reader currently silences malformed/truncated FASTQ
by relying only on parser::find_record_offsets and returning Ok(false) when
leftovers exist at EOF; update the logic in the block that sets self.offsets and
next_record_idx (and the similar branch around parse_single_record handling at
142-157) to detect incomplete or invalid trailing data and return
Err(std::io::ErrorKind::InvalidData.into()) instead of treating it as no-data:
specifically, after computing self.offsets (via parser::find_record_offsets) and
before returning Ok(...), validate the leftover bytes when self.at_eof or when
offsets indicate a partial record (e.g., call parse_single_record or otherwise
validate the final offset/length vs. self.valid) and convert any parse failure
or seq/qual/plus-line mismatch into an Err(InvalidData) so malformed last
records are surfaced.
In `@src/commands/extract.rs`:
- Around line 1243-1249: The quality-sampling loop using SimdFastqReader
(temp_reader.next()) currently treats Some(Err(_)) as EOF and breaks, which
swallows FASTQ parse errors and can misreport "no records"; change the match arm
for Some(Err(e)) to propagate the parse error instead of breaking—e.g., return
or propagate an appropriate Err constructed from the error (preserving the
original error context) from the function that contains the loop (the function
that owns sample_quals and calls open_fastq_reader), so malformed input surfaces
instead of being treated as end-of-file.
---
Nitpick comments:
In `@crates/fgumi-simd-fastq/benches/fastq_parsing.rs`:
- Around line 91-105: Replace the hardcoded workstation path by reading an
opt-in environment variable (e.g. REAL_FASTQ_PATH) and only run the real-data
benchmarks when that env var is set and points to an existing file;
specifically, replace usage of the literal real_fastq_path with a call to
std::env::var("REAL_FASTQ_PATH").ok().and_then(|p| if Path::new(&p).exists() {
Some(p) } else { None }), then read the file and call group.throughput and
group.bench_with_input as before using find_record_offsets and
find_record_offsets_memchr so the benchmark is portable but still opt-in.
In `@crates/fgumi-simd-fastq/src/lexer.rs`:
- Around line 513-522: Replace the manual loop in the test function
test_simd_newlines_every_position with an rstest parameterized test: add the
#[rstest] attribute to the test, change it to accept a parameter pos: usize, and
provide pos values covering 0..64 (so each position becomes an individual case);
inside the test body keep the same setup using lex_block and lex_block_scalar
and the assert_eq!(simd_result, scalar_result, "Mismatch at position {pos}"); do
the same refactor for the similar sweep test around lines 551-571 so each pos is
a separate rstest case (referencing the same function names lex_block and
lex_block_scalar to locate the code).
In `@crates/fgumi-simd-fastq/src/parser.rs`:
- Around line 190-327: Add property-based tests using proptest to fuzz the FASTQ
parser paths: add proptest to dev-dependencies and create a new test that
generates arbitrary byte buffers and asserts invariants for find_record_offsets
and parse_records — e.g., offsets from find_record_offsets are nondecreasing,
start with 0, each offset < = buffer.len(), if the final offset == buffer.len()
then parse_records yields only complete records whose concatenation matches the
original complete-record bytes, and parsed record fields (name, sequence,
quality) obey expected separators; target both find_record_offsets and
parse_records when writing strategies (including random ASCII/newline
distributions and small mutations) to catch SIMD/misalignment issues and
boundary-spanning records.
In `@src/lib/fastq.rs`:
- Around line 326-328: The code is cloning record.name/sequence/quality into
from_record_with_structure(), causing a second full copy; update the call and
the from_record_with_structure()/FastqSet constructors to take ownership of the
Vec<u8> produced by SimdFastqReader (e.g., accept name: Vec<u8>, sequence:
Vec<u8>, quality: Vec<u8> or use mem::take/into_owned on the record) instead of
borrowing &record.name etc., so you move the buffers directly from the
SimdFastqReader record into the FastqSet/segment without an extra clone; adjust
signatures and call sites accordingly (from_record_with_structure, FastqSet
fields) to reflect owned Vec<u8> consumption.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3689e861-96ba-4384-9d3b-f521aeca7eb1
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomlcrates/fgumi-simd-fastq/Cargo.tomlcrates/fgumi-simd-fastq/benches/fastq_parsing.rscrates/fgumi-simd-fastq/src/bitmask.rscrates/fgumi-simd-fastq/src/lexer.rscrates/fgumi-simd-fastq/src/lib.rscrates/fgumi-simd-fastq/src/parser.rscrates/fgumi-simd-fastq/src/reader.rssrc/commands/extract.rssrc/lib/fastq.rssrc/lib/unified_pipeline/fastq.rs
nh13
force-pushed
the
feat/nh/simd-fastq-parser
branch
from
March 23, 2026 16:32
3eb3efc to
51dc35e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/fgumi-simd-fastq/benches/fastq_parsing.rs (1)
92-95: Avoid machine-local hardcoded benchmark input paths.Line 92 hardcodes a host-specific path. Prefer an env var so others can run the same benchmark without editing code.
Proposed refactor
- let real_fastq_path = "/Volumes/scratch-00001/work/rebgzf_test.fastq"; - if std::path::Path::new(real_fastq_path).exists() { - let data = std::fs::read(real_fastq_path).unwrap(); + if let Ok(real_fastq_path) = std::env::var("FGUMI_BENCH_FASTQ") { + let data = std::fs::read(&real_fastq_path) + .expect("failed to read FGUMI_BENCH_FASTQ benchmark input"); let data_size = data.len(); group.throughput(Throughput::Bytes(data_size as u64));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-simd-fastq/benches/fastq_parsing.rs` around lines 92 - 95, Replace the hardcoded path assigned to real_fastq_path with an environment-driven lookup (e.g., std::env::var("FASTQ_BENCH_PATH").ok()) and handle the absence gracefully: if the env var is set use that path for reading into data and computing data_size, otherwise either fall back to a bundled test fixture or return/skip the benchmark; update the code that uses real_fastq_path, data, and data_size (the variables in this bench function) to handle Result/Option error cases instead of unwrapping so CI and other machines won’t depend on a machine-local file.src/lib/unified_pipeline/fastq.rs (1)
449-466: Clarify the boundary validation contract.
find_record_offsets()correctly identifies record boundaries at every 4th newline (standard FASTQ structure), but it doesn't validate that record starts contain@—that check happens later inparse_single_fastq_record(). If this deferred validation is intentional, document it in the function contract; otherwise, add a cheap byte-check here to fail early on malformed chunks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/unified_pipeline/fastq.rs` around lines 449 - 466, The function find_fastq_boundaries_inplace currently relies on fgumi_simd_fastq::find_record_offsets to locate 4-line FASTQ record boundaries but does not validate that each record start byte is '@' (that validation is done later in parse_single_fastq_record); either document this deferred-validation contract in the function docstring or add a cheap early sanity check: iterate the returned offsets from find_record_offsets and for each offset < last_offset verify data[offset] == b'@' and fail early (return an error/empty result or propagate a suitable error) on the first mismatch; reference find_fastq_boundaries_inplace, fgumi_simd_fastq::find_record_offsets, and parse_single_fastq_record when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/fgumi-simd-fastq/benches/fastq_parsing.rs`:
- Around line 122-166: The SIMD benches currently differ from the seq_io
baseline by not accessing record fields and by treating SimdFastqReader items as
already-unwrapped; update the parse_records and SimdFastqReader benchmarks to
mirror SeqIoReader: iterate over the reader (or parse_records iterator) and for
each item match/unwrap the io::Result, call std::hint::black_box on rec.head(),
rec.seq(), and rec.qual() to prevent DCE, and only increment count for
successful records (or panic/unwrap on Err to surface errors) so the comparison
is fair and SimdFastqReader errors are handled explicitly.
In `@src/lib/fastq.rs`:
- Around line 18-25: The module and struct Rustdoc examples construct
SimdFastqReader without trait-object coercion but ReadSetIterator expects a
boxed trait object; update both examples to wrap the BufReader in Box<dyn
BufRead + Send> when creating the reader (e.g., use
SimdFastqReader::new(Box::new(BufReader::new(file)) as Box<dyn BufRead + Send>))
so the reader type matches ReadSetIterator::new(rs, reader, vec![]); ensure both
the top-level example (where ReadStructure::from_str is used) and the
struct-level example are changed to use Box<dyn BufRead + Send> per the tests
that cast the reader.
---
Nitpick comments:
In `@crates/fgumi-simd-fastq/benches/fastq_parsing.rs`:
- Around line 92-95: Replace the hardcoded path assigned to real_fastq_path with
an environment-driven lookup (e.g., std::env::var("FASTQ_BENCH_PATH").ok()) and
handle the absence gracefully: if the env var is set use that path for reading
into data and computing data_size, otherwise either fall back to a bundled test
fixture or return/skip the benchmark; update the code that uses real_fastq_path,
data, and data_size (the variables in this bench function) to handle
Result/Option error cases instead of unwrapping so CI and other machines won’t
depend on a machine-local file.
In `@src/lib/unified_pipeline/fastq.rs`:
- Around line 449-466: The function find_fastq_boundaries_inplace currently
relies on fgumi_simd_fastq::find_record_offsets to locate 4-line FASTQ record
boundaries but does not validate that each record start byte is '@' (that
validation is done later in parse_single_fastq_record); either document this
deferred-validation contract in the function docstring or add a cheap early
sanity check: iterate the returned offsets from find_record_offsets and for each
offset < last_offset verify data[offset] == b'@' and fail early (return an
error/empty result or propagate a suitable error) on the first mismatch;
reference find_fastq_boundaries_inplace, fgumi_simd_fastq::find_record_offsets,
and parse_single_fastq_record when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba817fc8-885e-468f-a42b-a45a77bc7cb0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomlcrates/fgumi-simd-fastq/Cargo.tomlcrates/fgumi-simd-fastq/benches/fastq_parsing.rscrates/fgumi-simd-fastq/src/bitmask.rscrates/fgumi-simd-fastq/src/lexer.rscrates/fgumi-simd-fastq/src/lib.rscrates/fgumi-simd-fastq/src/parser.rscrates/fgumi-simd-fastq/src/reader.rssrc/commands/extract.rssrc/lib/fastq.rssrc/lib/unified_pipeline/fastq.rs
✅ Files skipped from review due to trivial changes (5)
- crates/fgumi-simd-fastq/Cargo.toml
- crates/fgumi-simd-fastq/src/bitmask.rs
- crates/fgumi-simd-fastq/src/lib.rs
- src/commands/extract.rs
- crates/fgumi-simd-fastq/src/lexer.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- Cargo.toml
- crates/fgumi-simd-fastq/src/parser.rs
- crates/fgumi-simd-fastq/src/reader.rs
nh13
force-pushed
the
feat/nh/simd-fastq-parser
branch
from
March 23, 2026 16:50
51dc35e to
d57748c
Compare
…q crate Reimplement the Helicase paper's SIMD FASTQ parsing technique in a new fgumi-simd-fastq crate. Processes 64 bytes at a time through SIMD registers (NEON on aarch64, AVX2 on x86_64), classifying newline characters via bitmask operations and finding record boundaries without per-byte branching. Two lexer modes: - lex_block(): newlines only (~18 GiB/s), used for boundary detection - lex_block_full(): newlines + ACGT + 2-bit encoding (BitEnc-compatible), ready for future fused pipeline Replaces memchr-based boundary finding (3.3x faster on real FASTQ data) and seq_io dependency with SimdFastqReader.
nh13
force-pushed
the
feat/nh/simd-fastq-parser
branch
from
March 23, 2026 16:53
d57748c to
af4b066
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/fgumi-simd-fastq/src/reader.rs (1)
142-148:⚠️ Potential issue | 🟡 MinorMalformed record causes panic, not
Err.
parse_single_recordusesassert!(panics on missing@or wrong newline count). Sincefind_record_offsetsonly counts newlines without validating structure, malformed data with 4 newlines (e.g., missing@) will panic here instead of returningErr(InvalidData).Consider wrapping in
catch_unwindor using a fallible parse variant:Suggested approach
- let borrowed = parse_single_record(&self.buffer[start..end]); - return Some(Ok(OwnedFastqRecord { + let record_data = &self.buffer[start..end]; + if record_data.is_empty() || record_data[0] != b'@' { + return Some(Err(io::Error::new( + io::ErrorKind::InvalidData, + "FASTQ record must start with @", + ))); + } + let borrowed = parse_single_record(record_data); + return Some(Ok(OwnedFastqRecord {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-simd-fastq/src/reader.rs` around lines 142 - 148, parse_single_record can panic (it uses assert!) when given malformed slices, so replace the direct call inside the record-return branch with a fallible parse or a panic-safe wrapper: either implement/Call a non-panicking variant (e.g., parse_single_record_fallible) that returns Result<ParsedRecord, InvalidData> and map that to Some(Err(InvalidData)), or wrap the existing parse_single_record(&self.buffer[start..end]) in std::panic::catch_unwind and convert any panic into an Err(InvalidData). Ensure you still construct OwnedFastqRecord only on Ok and return Some(Err(...)) on failure; reference parse_single_record, find_record_offsets, OwnedFastqRecord and the slice self.buffer[start..end] to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/fgumi-simd-fastq/src/reader.rs`:
- Around line 142-148: parse_single_record can panic (it uses assert!) when
given malformed slices, so replace the direct call inside the record-return
branch with a fallible parse or a panic-safe wrapper: either implement/Call a
non-panicking variant (e.g., parse_single_record_fallible) that returns
Result<ParsedRecord, InvalidData> and map that to Some(Err(InvalidData)), or
wrap the existing parse_single_record(&self.buffer[start..end]) in
std::panic::catch_unwind and convert any panic into an Err(InvalidData). Ensure
you still construct OwnedFastqRecord only on Ok and return Some(Err(...)) on
failure; reference parse_single_record, find_record_offsets, OwnedFastqRecord
and the slice self.buffer[start..end] to locate and update the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f2020c1-7d91-4810-9976-30a371dab7d9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomlcrates/fgumi-simd-fastq/Cargo.tomlcrates/fgumi-simd-fastq/benches/fastq_parsing.rscrates/fgumi-simd-fastq/src/bitmask.rscrates/fgumi-simd-fastq/src/lexer.rscrates/fgumi-simd-fastq/src/lib.rscrates/fgumi-simd-fastq/src/parser.rscrates/fgumi-simd-fastq/src/reader.rssrc/commands/extract.rssrc/lib/fastq.rssrc/lib/unified_pipeline/fastq.rs
✅ Files skipped from review due to trivial changes (3)
- crates/fgumi-simd-fastq/Cargo.toml
- crates/fgumi-simd-fastq/src/bitmask.rs
- crates/fgumi-simd-fastq/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/fgumi-simd-fastq/benches/fastq_parsing.rs
- src/lib/fastq.rs
- crates/fgumi-simd-fastq/src/lexer.rs
Merged
2 tasks
This branch was previously deployed
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
1 participant
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
fgumi-simd-fastqcrate implementing Helicase-style SIMD FASTQ parsingseq_iodependency withSimdFastqReaderand SIMD-accelerated boundary findingTwo lexer modes
lex_block()— newline-only detection (~18 GiB/s), used for record boundary findinglex_block_full()— newlines + ACGT classification + 2-bit encoding (BitEnc-compatible A=0/C=1/G=2/T=3), exported for future fused pipeline useIntegration
find_fastq_boundaries_inplace()in the unified pipeline delegates to SIMDfind_record_offsets()— 3.3x faster than the previous memchr-based approach on real FASTQ dataReadSetIteratorusesSimdFastqReaderinstead ofseq_io::fastq::Readerseq_iodependency removedBenchmark results (Apple Silicon, NEON)
Boundary finding (in-memory,
find_record_offsets):Full record parsing (
parse_records/SimdFastqReadervsseq_io):Full record parsing is slower than seq_io because the SIMD pass finds record boundaries but discards internal newline positions — the per-record field extraction re-scans for newlines. A future optimization (recording internal newlines during the SIMD pass) would close this gap.
End-to-end
fgumi extractshows no measurable wall-clock difference on compressed FASTQ inputs where decompression dominates. The boundary-finding speedup matters more in the planned fused pipeline where parsing is a larger fraction of total work.Test plan
fgumi-simd-fastq(lexer, parser, reader, SIMD-vs-scalar consistency)cargo ci-fmtandcargo ci-lintclean