Skip to content

Commit dcff5a3

Browse files
committed
Fuse read base restoration, CIGAR building, and NM/MD computation into single pass.
Previously CRAM decode performed 3-4 separate passes over each record's features: restoreReadBases (2 sub-passes), getCigarForReadFeatures, and calculateMdAndNm (via a separately-derived CIGAR). The new restoreBasesAndTags method in CRAMRecordReadFeatures does all of this in a single iteration, simultaneously filling bases, building CIGAR elements, and accumulating the MD string and NM count. Base normalization (toBamReadBasesInPlace) is also folded in. The CIGAR is cached on CRAMCompressionRecord so toSAMRecord() doesn't need to recompute it. Handles edge cases: reads extending beyond the reference boundary, unmapped reads, and the no-features fast path. Made SequenceUtil.bamReadBaseLookup public for use by the fused method. Benchmarked: 38% cumulative decode speedup (180s -> 111s on 51.7M records).
1 parent cc0f010 commit dcff5a3

5 files changed

Lines changed: 795 additions & 147 deletions

File tree

src/main/java/htsjdk/samtools/cram/structure/CRAMCompressionRecord.java

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public class CRAMCompressionRecord {
8484
// the contents hasher doesn't handle nulls
8585
private byte[] readBases;
8686
private byte[] qualityScores;
87+
private Cigar cachedCigar; // populated by restoreBasesAndTags, used by toSAMRecord
8788
private MutableInt tagIdsIndex = new MutableInt(0);
8889

8990
//mate info
@@ -346,7 +347,7 @@ public SAMRecord toSAMRecord(final SAMFileHeader samFileHeader) {
346347
if (isSegmentUnmapped()) {
347348
samRecord.setCigarString(SAMRecord.NO_ALIGNMENT_CIGAR);
348349
} else {
349-
samRecord.setCigar(readFeatures.getCigarForReadFeatures(readLength));
350+
samRecord.setCigar(cachedCigar != null ? cachedCigar : readFeatures.getCigarForReadFeatures(readLength));
350351
}
351352

352353
if (samRecord.getReadPairedFlag()) {
@@ -515,11 +516,14 @@ public void restoreReadBases(CRAMReferenceRegion cramReferenceRegion, final Subs
515516
}
516517

517518
/**
518-
* Compute and store NM/MD tags from the reference region. Should be called after
519-
* {@link #restoreReadBases} while the cramReferenceRegion still covers this record's span.
520-
* Strips the internal cF tag if present (from htslib embed_ref=2 mode).
519+
* Fused single-pass method: restore read bases from the reference + read features, build the
520+
* CIGAR, and compute NM/MD tags, all in one iteration through the features. Replaces the
521+
* previous separate calls to {@code restoreReadBases} + {@code restoreNmAndMd}.
522+
*
523+
* <p>The CIGAR is cached on this record for use by {@link #toSAMRecord()}.
521524
*/
522-
void restoreNmAndMd(final CRAMReferenceRegion cramReferenceRegion) {
525+
void restoreBasesAndTags(final CRAMReferenceRegion cramReferenceRegion,
526+
final SubstitutionMatrix substitutionMatrix) {
523527
// Handle the cF internal tag from htslib's embed_ref=2 mode
524528
boolean suppressMD = false;
525529
boolean suppressNM = false;
@@ -535,18 +539,7 @@ void restoreNmAndMd(final CRAMReferenceRegion cramReferenceRegion) {
535539
}
536540
}
537541

538-
if (isSegmentUnmapped() || readLength == 0 || readBases == null || readBases.length == 0 ||
539-
readFeatures == null || cramReferenceRegion.getCurrentReferenceBases() == null) {
540-
return;
541-
}
542-
543-
final int refOffset = cramReferenceRegion.getRegionStart();
544-
final int startInRef = alignmentStart - 1 - refOffset;
545-
if (startInRef < 0 || startInRef >= cramReferenceRegion.getCurrentReferenceBases().length) {
546-
return;
547-
}
548-
549-
// Check if NM/MD are already present in tags
542+
// Determine if MD/NM computation is needed
550543
boolean hasNM = false;
551544
boolean hasMD = false;
552545
if (tags != null) {
@@ -555,24 +548,36 @@ void restoreNmAndMd(final CRAMReferenceRegion cramReferenceRegion) {
555548
if ("MD".equals(tag.getKey())) hasMD = true;
556549
}
557550
}
558-
559551
final boolean needMD = !hasMD && !suppressMD;
560552
final boolean needNM = !hasNM && !suppressNM;
561553

562-
if (needMD || needNM) {
563-
final Cigar cigar = readFeatures.getCigarForReadFeatures(readLength);
564-
final htsjdk.samtools.util.Tuple<String, Integer> result =
565-
htsjdk.samtools.util.SequenceUtil.calculateMdAndNm(
566-
cigar.getCigarElements(),
567-
readBases,
568-
cramReferenceRegion.getCurrentReferenceBases(),
569-
cramReferenceRegion.getRegionStart(),
570-
alignmentStart);
554+
final boolean computeMdNm = (needMD || needNM) &&
555+
readLength > 0 &&
556+
readFeatures != null &&
557+
cramReferenceRegion.getCurrentReferenceBases() != null;
558+
559+
final CRAMRecordReadFeatures.DecodeResult result = CRAMRecordReadFeatures.restoreBasesAndTags(
560+
readFeatures == null ? Collections.emptyList() : readFeatures.getReadFeaturesList(),
561+
isUnknownBases(),
562+
alignmentStart,
563+
readLength,
564+
cramReferenceRegion,
565+
substitutionMatrix,
566+
computeMdNm);
567+
568+
this.readBases = result.readBases;
569+
this.cachedCigar = result.cigar;
570+
571+
if (computeMdNm) {
571572
if (tags == null) {
572573
tags = new ArrayList<>(2);
573574
}
574-
if (needMD) tags.add(ReadTag.deriveTypeFromValue("MD", result.a));
575-
if (needNM) tags.add(ReadTag.deriveTypeFromValue("NM", result.b));
575+
if (needMD && result.mdString != null) {
576+
tags.add(ReadTag.deriveTypeFromValue("MD", result.mdString));
577+
}
578+
if (needNM && result.nmCount >= 0) {
579+
tags.add(ReadTag.deriveTypeFromValue("NM", result.nmCount));
580+
}
576581
}
577582
}
578583

0 commit comments

Comments
 (0)