From 0b5e3d85c703a220b7b26ec0bb56e54f4fc936e6 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Dec 2018 14:17:26 -0500 Subject: [PATCH 1/3] Replaced several for-each loops in VariantContext.Make() codepath with slightly faster alternatives where it made a difference for the Haplotype Caller. --- .../variant/variantcontext/Genotype.java | 22 +++++++++-------- .../variantcontext/VariantContext.java | 24 ++++++++++++------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/main/java/htsjdk/variant/variantcontext/Genotype.java b/src/main/java/htsjdk/variant/variantcontext/Genotype.java index 8d781a0ada..deb0e54efb 100644 --- a/src/main/java/htsjdk/variant/variantcontext/Genotype.java +++ b/src/main/java/htsjdk/variant/variantcontext/Genotype.java @@ -197,31 +197,33 @@ public GenotypeType getType() { protected GenotypeType determineType() { // TODO -- this code is slow and could be optimized for the diploid case final List alleles = getAlleles(); - if ( alleles.isEmpty() ) + if ( alleles.isEmpty() ) { return GenotypeType.UNAVAILABLE; + } boolean sawNoCall = false, sawMultipleAlleles = false; - Allele observedAllele = null; + Allele firstAllele = null; - for ( final Allele allele : alleles ) { - if ( allele.isNoCall() ) + for ( int i = 0; i < alleles.size(); i++ ) { + final Allele allele = alleles.get(i); + if ( allele.isNoCall() ) { sawNoCall = true; - else if ( observedAllele == null ) - observedAllele = allele; - else if ( !allele.equals(observedAllele) ) + } else if ( firstAllele == null ) { + firstAllele = allele; + } else if ( !allele.equals(firstAllele) ) sawMultipleAlleles = true; } if ( sawNoCall ) { - if ( observedAllele == null ) + if ( firstAllele == null ) return GenotypeType.NO_CALL; return GenotypeType.MIXED; } - if ( observedAllele == null ) + if ( firstAllele == null ) throw new IllegalStateException("BUG: there are no alleles present in this genotype but the alleles list is not null"); - return sawMultipleAlleles ? GenotypeType.HET : observedAllele.isReference() ? GenotypeType.HOM_REF : GenotypeType.HOM_VAR; + return sawMultipleAlleles ? GenotypeType.HET : firstAllele.isReference() ? GenotypeType.HOM_REF : GenotypeType.HOM_VAR; } /** diff --git a/src/main/java/htsjdk/variant/variantcontext/VariantContext.java b/src/main/java/htsjdk/variant/variantcontext/VariantContext.java index 9fc6c7b839..abc2c1e3c2 100644 --- a/src/main/java/htsjdk/variant/variantcontext/VariantContext.java +++ b/src/main/java/htsjdk/variant/variantcontext/VariantContext.java @@ -822,8 +822,8 @@ private boolean hasAllele(final Allele allele, final boolean ignoreRefState, fin return true; final List allelesToConsider = considerRefAllele ? getAlleles() : getAlternateAlleles(); - for ( Allele a : allelesToConsider ) { - if ( a.equals(allele, ignoreRefState) ) + for ( int i = 0; i < allelesToConsider.size(); i++) { + if ( allelesToConsider.get(i).equals(allele, ignoreRefState) ) return true; } @@ -1352,9 +1352,9 @@ private void validateAlleles() { private void validateGenotypes() { if ( this.genotypes == null ) throw new IllegalStateException("Genotypes is null"); - for ( final Genotype g : this.genotypes ) { - if ( g.isAvailable() ) { - for ( Allele gAllele : g.getAlleles() ) { + for ( int i = 0; i < genotypes.size(); i++ ) { + if ( genotypes.get(i).isAvailable() ) { + for ( Allele gAllele : genotypes.get(i).getAlleles() ) { if ( ! hasAllele(gAllele) && gAllele.isCalled() ) throw new IllegalStateException("Allele in genotype " + gAllele + " not in the variant context " + alleles); } @@ -1484,9 +1484,10 @@ private static List makeAlleles(Collection alleles) { boolean sawRef = false; for ( final Allele a : alleles ) { - for ( final Allele b : alleleList ) { - if ( a.equals(b, true) ) + for ( int i = 0; i < alleleList.size(); i++ ) { + if ( a.equals(alleleList.get(i), true) ) { throw new IllegalArgumentException("Duplicate allele added to VariantContext: " + a); + } } // deal with the case where the first allele isn't the reference @@ -1696,8 +1697,13 @@ public boolean hasSymbolicAlleles() { return hasSymbolicAlleles(getAlleles()); } - public static boolean hasSymbolicAlleles(final List alleles) { - return alleles.stream().anyMatch(Allele::isSymbolic); + public static boolean hasSymbolicAlleles( final List alleles ) { + for (int i = 0; i < alleles.size(); i++ ) { + if (alleles.get(i).isSymbolic()) { + return true; + } + } + return false; } public Allele getAltAlleleWithHighestAlleleCount() { From b68d1ac2600d04fcd0cac9ff4da7c5950fd4216c Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Dec 2018 12:39:39 -0500 Subject: [PATCH 2/3] responded to first round of comments --- .../htsjdk/variant/variantcontext/Genotype.java | 14 +++++++------- .../variant/variantcontext/VariantContext.java | 10 +++++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/main/java/htsjdk/variant/variantcontext/Genotype.java b/src/main/java/htsjdk/variant/variantcontext/Genotype.java index deb0e54efb..a0c80c2e22 100644 --- a/src/main/java/htsjdk/variant/variantcontext/Genotype.java +++ b/src/main/java/htsjdk/variant/variantcontext/Genotype.java @@ -202,28 +202,28 @@ protected GenotypeType determineType() { } boolean sawNoCall = false, sawMultipleAlleles = false; - Allele firstAllele = null; + Allele firstCallAllele = null; for ( int i = 0; i < alleles.size(); i++ ) { final Allele allele = alleles.get(i); if ( allele.isNoCall() ) { sawNoCall = true; - } else if ( firstAllele == null ) { - firstAllele = allele; - } else if ( !allele.equals(firstAllele) ) + } else if ( firstCallAllele == null ) { + firstCallAllele = allele; + } else if ( !allele.equals(firstCallAllele) ) sawMultipleAlleles = true; } if ( sawNoCall ) { - if ( firstAllele == null ) + if ( firstCallAllele == null ) return GenotypeType.NO_CALL; return GenotypeType.MIXED; } - if ( firstAllele == null ) + if ( firstCallAllele == null ) throw new IllegalStateException("BUG: there are no alleles present in this genotype but the alleles list is not null"); - return sawMultipleAlleles ? GenotypeType.HET : firstAllele.isReference() ? GenotypeType.HOM_REF : GenotypeType.HOM_VAR; + return sawMultipleAlleles ? GenotypeType.HET : firstCallAllele.isReference() ? GenotypeType.HOM_REF : GenotypeType.HOM_VAR; } /** diff --git a/src/main/java/htsjdk/variant/variantcontext/VariantContext.java b/src/main/java/htsjdk/variant/variantcontext/VariantContext.java index abc2c1e3c2..54bc38d73f 100644 --- a/src/main/java/htsjdk/variant/variantcontext/VariantContext.java +++ b/src/main/java/htsjdk/variant/variantcontext/VariantContext.java @@ -1353,8 +1353,11 @@ private void validateGenotypes() { if ( this.genotypes == null ) throw new IllegalStateException("Genotypes is null"); for ( int i = 0; i < genotypes.size(); i++ ) { - if ( genotypes.get(i).isAvailable() ) { - for ( Allele gAllele : genotypes.get(i).getAlleles() ) { + Genotype genotype = genotypes.get(i); + if ( genotype.isAvailable() ) { + final List alleles = genotype.getAlleles(); + for ( int j = 0; j < alleles.size(); j++ ) { + final Allele gAllele = alleles.get(j); if ( ! hasAllele(gAllele) && gAllele.isCalled() ) throw new IllegalStateException("Allele in genotype " + gAllele + " not in the variant context " + alleles); } @@ -1698,7 +1701,8 @@ public boolean hasSymbolicAlleles() { } public static boolean hasSymbolicAlleles( final List alleles ) { - for (int i = 0; i < alleles.size(); i++ ) { + int size = alleles.size(); + for (int i = 0; i < size; i++ ) { if (alleles.get(i).isSymbolic()) { return true; } From 363a69e3af48bc008f9b4c330295db3ef426edbb Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Dec 2018 13:22:12 -0500 Subject: [PATCH 3/3] sorcery --- .../variant/variantcontext/VariantContext.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/htsjdk/variant/variantcontext/VariantContext.java b/src/main/java/htsjdk/variant/variantcontext/VariantContext.java index 54bc38d73f..b10de475ea 100644 --- a/src/main/java/htsjdk/variant/variantcontext/VariantContext.java +++ b/src/main/java/htsjdk/variant/variantcontext/VariantContext.java @@ -822,8 +822,9 @@ private boolean hasAllele(final Allele allele, final boolean ignoreRefState, fin return true; final List allelesToConsider = considerRefAllele ? getAlleles() : getAlternateAlleles(); - for ( int i = 0; i < allelesToConsider.size(); i++) { - if ( allelesToConsider.get(i).equals(allele, ignoreRefState) ) + for (int i = 0, allelesToConsiderSize = allelesToConsider.size(); i < allelesToConsiderSize; i++) { + Allele anAllelesToConsider = allelesToConsider.get(i); + if (anAllelesToConsider.equals(allele, ignoreRefState)) return true; } @@ -1356,7 +1357,7 @@ private void validateGenotypes() { Genotype genotype = genotypes.get(i); if ( genotype.isAvailable() ) { final List alleles = genotype.getAlleles(); - for ( int j = 0; j < alleles.size(); j++ ) { + for ( int j = 0, size = alleles.size(); j < size; j++ ) { final Allele gAllele = alleles.get(j); if ( ! hasAllele(gAllele) && gAllele.isCalled() ) throw new IllegalStateException("Allele in genotype " + gAllele + " not in the variant context " + alleles); @@ -1487,8 +1488,8 @@ private static List makeAlleles(Collection alleles) { boolean sawRef = false; for ( final Allele a : alleles ) { - for ( int i = 0; i < alleleList.size(); i++ ) { - if ( a.equals(alleleList.get(i), true) ) { + for (int i = 0, alleleListSize = alleleList.size(); i < alleleListSize; i++) { + if (a.equals(alleleList.get(i), true)) { throw new IllegalArgumentException("Duplicate allele added to VariantContext: " + a); } } @@ -1701,8 +1702,7 @@ public boolean hasSymbolicAlleles() { } public static boolean hasSymbolicAlleles( final List alleles ) { - int size = alleles.size(); - for (int i = 0; i < size; i++ ) { + for (int i = 0, size = alleles.size(); i < size; i++ ) { if (alleles.get(i).isSymbolic()) { return true; }