Skip to content

perf(duplex): partition strand groups by move instead of cloning - #424

Merged
nh13 merged 1 commit into
feat-runallfrom
feat-runall-26-duplex-strand-partition
Jun 17, 2026
Merged

nh13 merged 1 commit into
feat-runallfrom
feat-runall-26-duplex-strand-partition

Conversation

@nh13

@nh13 nh13 commented Jun 16, 2026

Copy link
Copy Markdown
Member

Wave 1 perf cleanup (#3 of the feat-runall review series).

What

process_group split the alignment-filtered filtered_xs / filtered_ys back into the four strand groups (AB-R1, AB-R2, BA-R1, BA-R2) with four iter().filter(...).cloned().collect() passes — deep-cloning every surviving SourceRead (its bases, quals, and two cigar vecs) once per duplex template.

The two splits of each vec are disjoint partitions on a single flag bit (FIRST_SEGMENT set vs clear), so this moves each read into its group with into_iter().partition(...) in one pass, no clones.

Behavior

Unchanged. The partition predicates map 1:1 to the original four filters, and partition preserves per-group iteration order, so consensus output is identical.

Verification

  • cargo ci-fmt / cargo ci-lint (clippy pedantic) clean.
  • cargo nextest -E 'test(duplex)' — 91 tests pass, including runall_duplex_record_stream_is_deterministic, the fgbio-parity duplex tests, and test_duplex_metrics_command::*.
  • CodeRabbit CLI (--agent) and a CodeRabbit-style self-review: 0 findings.

`process_group` split the alignment-filtered `filtered_xs` / `filtered_ys`
back into the four strand groups (AB-R1, AB-R2, BA-R1, BA-R2) with four
`iter().filter(...).cloned().collect()` passes, deep-cloning every surviving
`SourceRead` (its bases, quals, and two cigar vecs) once per duplex template.

The two splits of each vec are disjoint partitions on a single flag bit
(`FIRST_SEGMENT` set vs clear), so use `into_iter().partition(...)` to move
each read into its group in one pass with no clones. Partition preserves
per-group order, so the consensus output is unchanged.
@nh13
nh13 temporarily deployed to github-actions June 16, 2026 21:05 — with GitHub Actions Inactive
@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (feat-runall@acf756f). Learn more about missing BASE report.

Additional details and impacted files
@@              Coverage Diff               @@
##             feat-runall     #424   +/-   ##
==============================================
  Coverage               ?   93.31%           
==============================================
  Files                  ?      139           
  Lines                  ?    54920           
  Branches               ?        0           
==============================================
  Hits                   ?    51249           
  Misses                 ?     3671           
  Partials               ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nh13

nh13 commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

1 similar comment
@nh13

nh13 commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 17, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3aa40a30-d2eb-4fdd-957f-2eecb92d70d3

📥 Commits

Reviewing files that changed from the base of the PR and between acf756f and e8da2ce.

📒 Files selected for processing (1)
  • crates/fgumi-consensus/src/duplex_caller.rs

📝 Walkthrough

Walkthrough

In process_group within duplex_caller.rs, the splitting of filtered_xs and filtered_ys into the four AB/BA R1/R2 groups is changed from iter().filter(...).cloned().collect() to into_iter().partition(...). The partitioning logic based on the FIRST_SEGMENT flag is unchanged; only the ownership model changes, removing per-element cloning.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately describes the main optimization: replacing cloning with move-based partitioning in the duplex strand group processing.
Description check ✅ Passed Description clearly relates to the changeset, detailing the performance optimization from clone-based to move-based partitioning, including behavior verification and test results.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-runall-26-duplex-strand-partition

Comment @coderabbitai help to get the list of available commands and usage tips.

@nh13

nh13 commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 17, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 17, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13
nh13 merged commit c449468 into feat-runall Jun 17, 2026
10 checks passed
@nh13
nh13 deleted the feat-runall-26-duplex-strand-partition branch June 17, 2026 16:24
nh13 added a commit that referenced this pull request Jun 18, 2026
`process_group` split the alignment-filtered `filtered_xs` / `filtered_ys`
back into the four strand groups (AB-R1, AB-R2, BA-R1, BA-R2) with four
`iter().filter(...).cloned().collect()` passes, deep-cloning every surviving
`SourceRead` (its bases, quals, and two cigar vecs) once per duplex template.

The two splits of each vec are disjoint partitions on a single flag bit
(`FIRST_SEGMENT` set vs clear), so use `into_iter().partition(...)` to move
each read into its group in one pass with no clones. Partition preserves
per-group order, so the consensus output is unchanged.

This branch was previously deployed

1 inactive deployment
github-actions — e8da2ce1 Deployed Jun 16, 2026 by nh13 via coverage #1661
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant