From b2aa974228b87c7aefc1f637fcae8ca30a920c80 Mon Sep 17 00:00:00 2001 From: kiramt Date: Thu, 5 Oct 2017 15:32:17 +0100 Subject: [PATCH] JAL-2674 Rationalising iterators and retesting --- src/jalview/datamodel/HiddenColumns.java | 504 ++++++++++++++++-------------- 1 file changed, 263 insertions(+), 241 deletions(-) diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index cd3183f..41bd16b 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -129,15 +129,16 @@ public class HiddenColumns { LOCK.readLock().lock(); StringBuilder regionBuilder = new StringBuilder(); - if (hiddenColumns != null) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - for (int[] range : hiddenColumns) - { - regionBuilder.append(delimiter).append(range[0]).append(between) + int[] range = it.next(); + regionBuilder.append(delimiter).append(range[0]).append(between) .append(range[1]); + if (!it.hasNext()) + { + regionBuilder.deleteCharAt(0); } - - regionBuilder.deleteCharAt(0); } return regionBuilder.toString(); } finally @@ -157,13 +158,13 @@ public class HiddenColumns { LOCK.readLock().lock(); int size = 0; - if (hasHiddenColumns()) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - for (int[] range : hiddenColumns) - { - size += range[1] - range[0] + 1; - } + int[] range = it.next(); + size += range[1] - range[0] + 1; } + return size; } finally { @@ -218,10 +219,13 @@ public class HiddenColumns { return false; } - int i = 0; - for (int[] thisRange : hiddenColumns) + + Iterator it = new LocalBoundedHiddenColsIterator(); + Iterator thatit = that.iterator(); + while (it.hasNext()) { - int[] thatRange = that.hiddenColumns.get(i++); + int[] thisRange = it.next(); + int[] thatRange = thatit.next(); if (thisRange[0] != thatRange[0] || thisRange[1] != thatRange[1]) { return false; @@ -247,17 +251,17 @@ public class HiddenColumns { LOCK.readLock().lock(); int result = column; - if (hiddenColumns != null) + + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - for (int i = 0; i < hiddenColumns.size(); i++) + int[] region = it.next(); + if (result >= region[0]) { - int[] region = hiddenColumns.get(i); - if (result >= region[0]) - { - result += region[1] - region[0] + 1; - } + result += region[1] - region[0] + 1; } } + return result; } finally { @@ -280,21 +284,22 @@ public class HiddenColumns { LOCK.readLock().lock(); int result = hiddenColumn; + int[] region = null; if (hiddenColumns != null) { - int index = 0; - int[] region; - do + Iterator it = new LocalBoundedHiddenColsIterator(0, + hiddenColumn); + while (it.hasNext()) { - region = hiddenColumns.get(index++); + region = it.next(); if (hiddenColumn > region[1]) { result -= region[1] + 1 - region[0]; } - } while ((hiddenColumn > region[1]) - && (index < hiddenColumns.size())); + } - if (hiddenColumn >= region[0] && hiddenColumn <= region[1]) + if (region != null && hiddenColumn >= region[0] + && hiddenColumn <= region[1]) { // Here the hidden column is within a region, so // we want to return the position of region[0]-1, adjusted for any @@ -606,14 +611,13 @@ public class HiddenColumns { LOCK.readLock().lock(); - if (hiddenColumns != null) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - for (int[] region : hiddenColumns) + int[] region = it.next(); + if (column >= region[0] && column <= region[1]) { - if (column >= region[0] && column <= region[1]) - { - return false; - } + return false; } } @@ -663,12 +667,22 @@ public class HiddenColumns { StringBuffer visibleSeq = new StringBuffer(); - Iterator blocks = new VisibleBlocksIterator(start, - end, false); + Iterator blocks = new VisibleContigsIterator(start, + end + 1, false); + while (blocks.hasNext()) { int[] block = blocks.next(); - visibleSeq.append(seqs[i].getSequence(block[0], block[1])); + if (blocks.hasNext()) + { + visibleSeq + .append(seqs[i].getSequence(block[0], block[1] + 1)); + } + else + { + visibleSeq + .append(seqs[i].getSequence(block[0], block[1])); + } } selections[i] = visibleSeq.toString(); @@ -711,7 +725,7 @@ public class HiddenColumns // Simply walk along the sequence whilst watching for hidden column // boundaries - Iterator regions = iterator(); + Iterator regions = new LocalBoundedHiddenColsIterator(); int hideStart = seq.getLength(); int hideEnd = -1; int visPrev = 0; @@ -816,23 +830,23 @@ public class HiddenColumns int w = 0; - Iterator blocks = new VisibleBlocksIterator(start, end, + Iterator blocks = new VisibleContigsIterator(start, end + 1, false); + int copylength; int annotationLength; while (blocks.hasNext()) { int[] block = blocks.next(); + annotationLength = block[1] - block[0] + 1; if (blocks.hasNext()) { - annotationLength = block[1] - block[0]; // copy just the visible segment of the annotation row copylength = annotationLength; } else { - annotationLength = block[1] - block[0] + 1; if (annotationLength + block[0] <= alignmentAnnotation.annotations.length) { // copy just the visible segment of the annotation row @@ -928,15 +942,13 @@ public class HiddenColumns try { LOCK.writeLock().lock(); - if (hiddenColumns != null) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - for (int i = 0; i < hiddenColumns.size(); i++) + int[] region = it.next(); + for (int j = region[0]; j < region[1] + 1; j++) { - int[] region = hiddenColumns.get(i); - for (int j = region[0]; j < region[1] + 1; j++) - { - sel.addElement(j); - } + sel.addElement(j); } } @@ -958,9 +970,10 @@ public class HiddenColumns try { LOCK.writeLock().lock(); - for (int i = 0; i < hiddenColumns.size(); i++) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - int[] region = hiddenColumns.get(i); + int[] region = it.next(); if (start == region[0]) { for (int j = region[0]; j < region[1] + 1; j++) @@ -971,7 +984,12 @@ public class HiddenColumns hiddenColumns.remove(region); break; } + else if (start < region[0]) + { + break; // passed all possible matching regions + } } + if (hiddenColumns.size() == 0) { hiddenColumns = null; @@ -1046,7 +1064,7 @@ public class HiddenColumns // of // preceding visible gaps // update hidden columns at the same time - Iterator regions = iterator(); + Iterator regions = new LocalBoundedHiddenColsIterator(); ArrayList newhidden = new ArrayList<>(); int numGapsBefore = 0; @@ -1151,13 +1169,12 @@ public class HiddenColumns { LOCK.readLock().lock(); int hashCode = 1; - if (hiddenColumns != null) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - for (int[] hidden : hiddenColumns) - { - hashCode = HASH_MULTIPLIER * hashCode + hidden[0]; - hashCode = HASH_MULTIPLIER * hashCode + hidden[1]; - } + int[] hidden = it.next(); + hashCode = HASH_MULTIPLIER * hashCode + hidden[0]; + hashCode = HASH_MULTIPLIER * hashCode + hidden[1]; } return hashCode; } finally @@ -1204,8 +1221,10 @@ public class HiddenColumns { return; } - for (int[] range : hiddenColumns) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { + int[] range = it.next(); inserts.set(range[0], range[1] + 1); } } finally @@ -1238,10 +1257,12 @@ public class HiddenColumns return new int[] { startPos, endPos }; } - for (int[] hiddenCol : hiddenColumns) + Iterator it = new LocalBoundedHiddenColsIterator(); + while (it.hasNext()) { - lowestRange = (hiddenCol[0] <= startPos) ? hiddenCol : lowestRange; - higestRange = (hiddenCol[1] >= endPos) ? hiddenCol : higestRange; + int[] range = it.next(); + lowestRange = (range[0] <= startPos) ? range : lowestRange; + higestRange = (range[1] >= endPos) ? range : higestRange; } if (lowestRange[0] == -1 && lowestRange[1] == -1) @@ -1284,15 +1305,15 @@ public class HiddenColumns int adjres = adjustForHiddenColumns(res); int[] reveal = null; - if (hiddenColumns != null) + Iterator it = new LocalBoundedHiddenColsIterator(adjres - 2, + adjres + 2); + while (it.hasNext()) { - for (int[] region : hiddenColumns) + int[] region = it.next(); + if (adjres + 1 == region[0] || adjres - 1 == region[1]) { - if (adjres + 1 == region[0] || adjres - 1 == region[1]) - { - reveal = region; - break; - } + reveal = region; + break; } } return reveal; @@ -1307,15 +1328,7 @@ public class HiddenColumns */ public Iterator iterator() { - if (hiddenColumns != null) - { - int last = hiddenColumns.get(hiddenColumns.size() - 1)[1]; - return new BoundedHiddenColsIterator(0, last, true); - } - else - { - return new BoundedHiddenColsIterator(0, 0, true); - } + return new BoundedHiddenColsIterator(); } /** @@ -1329,7 +1342,7 @@ public class HiddenColumns */ public Iterator getBoundedIterator(int start, int end) { - return new BoundedHiddenColsIterator(start, end, true); + return new BoundedHiddenColsIterator(start, end); } /** @@ -1347,8 +1360,8 @@ public class HiddenColumns } /** - * Return an iterator over visible columns between the given start and end - * boundaries + * Return an iterator over visible *columns* (not regions) between the given + * start and end boundaries * * @param start * first column (inclusive) @@ -1357,7 +1370,7 @@ public class HiddenColumns */ public Iterator getVisibleColsIterator(int start, int end) { - return new VisibleColsIterator(start, end, true); + return new VisibleColsIterator(start, end); } /** @@ -1371,6 +1384,7 @@ public class HiddenColumns */ public Iterator getVisContigsIterator(int start, int end) { + // return new VisibleBlocksIterator(start, end, true) return new VisibleContigsIterator(start, end, true); } @@ -1394,21 +1408,23 @@ public class HiddenColumns // TODO // we should really just convert start and end here with // adjustForHiddenColumns - // and then create a VisibleBlocksIterator + // and then create a VisibleContigsIterator // but without a cursor this will be horribly slow in some situations // ... so until then... return new VisibleBlocksVisBoundsIterator(start, end, true); } else { - return new VisibleBlocksIterator(start, end, true); + return new VisibleContigsIterator(start, end - 1, true); } } /** - * An iterator which iterates over hidden column regions in a range. + * A local iterator which iterates over hidden column regions in a range. + * Works with the actual hidden columns collection (so locking before use may + * be required). */ - private class BoundedHiddenColsIterator implements Iterator + private class LocalBoundedHiddenColsIterator implements Iterator { private int start; // start position to iterate from @@ -1418,13 +1434,25 @@ public class HiddenColumns private int currentPosition = 0; // current column in hiddenColumns - private int[] currentRegion; + private int[] nextRegion = null; - // whether to make a local copy of hiddenColumns - private final boolean useCopy; + LocalBoundedHiddenColsIterator(int lowerBound, int upperBound) + { + init(lowerBound, upperBound); + } - // local copy or reference to hiddenColumns - private List localHidden; + LocalBoundedHiddenColsIterator() + { + if (hiddenColumns != null) + { + int last = hiddenColumns.get(hiddenColumns.size() - 1)[1]; + init(0, last); + } + else + { + init(0, 0); + } + } /** * Construct an iterator over hiddenColums bounded at @@ -1438,21 +1466,115 @@ public class HiddenColumns * whether to make a local copy of hiddenColumns for iteration (set * to true if calling from outwith the HiddenColumns class) */ - BoundedHiddenColsIterator(int lowerBound, int upperBound, - boolean useCopyCols) + private void init(int lowerBound, int upperBound) { start = lowerBound; end = upperBound; - useCopy = useCopyCols; - try + if (hiddenColumns != null) { - if (useCopy) + // iterate until a region overlaps with [start,end] + currentPosition = 0; + while ((currentPosition < hiddenColumns.size()) + && (hiddenColumns.get(currentPosition)[1] < start)) { - // assume that if useCopy is false the calling code has locked - // hiddenColumns - LOCK.readLock().lock(); + currentPosition++; + } + if (currentPosition < hiddenColumns.size()) + { + nextRegion = hiddenColumns.get(currentPosition); } + } + } + + @Override + public boolean hasNext() + { + return (hiddenColumns != null) && (nextRegion != null) + && (nextRegion[0] <= end); + } + + @Override + public int[] next() + { + int[] region = nextRegion; + currentPosition++; + if (currentPosition < hiddenColumns.size()) + { + nextRegion = hiddenColumns.get(currentPosition); + } + else + { + nextRegion = null; + } + return region; + } + + } + + /** + * An iterator which iterates over hidden column regions in a range. Works + * with a copy of the hidden columns collection. + */ + private class BoundedHiddenColsIterator implements Iterator + { + private int start; // start position to iterate from + + private int end; // end position to iterate to + + // current index in hiddenColumns + private int currentPosition = 0; + + // current column in hiddenColumns + private int[] currentRegion; + + // local copy or reference to hiddenColumns + private List localHidden; + + BoundedHiddenColsIterator() + { + if (hiddenColumns != null) + { + int last = hiddenColumns.get(hiddenColumns.size() - 1)[1]; + init(0, last); + } + else + { + init(0, 0); + } + } + + /** + * Construct an iterator over hiddenColums bounded at + * [lowerBound,upperBound] + * + * @param lowerBound + * lower bound to iterate from + * @param upperBound + * upper bound to iterate to + */ + BoundedHiddenColsIterator(int lowerBound, int upperBound) + { + init(lowerBound, upperBound); + } + + /** + * Construct an iterator over hiddenColums bounded at + * [lowerBound,upperBound] + * + * @param lowerBound + * lower bound to iterate from + * @param upperBound + * upper bound to iterate to + */ + private void init(int lowerBound, int upperBound) + { + start = lowerBound; + end = upperBound; + + try + { + LOCK.readLock().lock(); if (hiddenColumns != null) { @@ -1471,25 +1593,17 @@ public class HiddenColumns while (i < hiddenColumns.size() && (hiddenColumns.get(i)[0] <= end)) { - int[] rh; - int[] cp; - rh = hiddenColumns.get(i); - if (rh != null) - { - cp = new int[rh.length]; - System.arraycopy(rh, 0, cp, 0, rh.length); - localHidden.add(cp); - } + int[] rh = hiddenColumns.get(i); + int[] cp = new int[2]; + System.arraycopy(rh, 0, cp, 0, rh.length); + localHidden.add(cp); i++; } } } finally { - if (useCopy) - { - LOCK.readLock().unlock(); - } + LOCK.readLock().unlock(); } } @@ -1613,6 +1727,13 @@ public class HiddenColumns } } + /** + * Iterator over the visible *columns* (not regions) as determined by the set + * of hidden columns. Uses a local copy of hidden columns. + * + * @author kmourao + * + */ private class VisibleColsIterator implements Iterator { private int last; @@ -1625,67 +1746,54 @@ public class HiddenColumns private int nexthiddenregion; - VisibleColsIterator(int firstcol, int lastcol, boolean useCopy) + VisibleColsIterator(int firstcol, int lastcol) { last = lastcol; current = firstcol; next = firstcol; nexthiddenregion = 0; - try - { - if (useCopy) - { - // assume that if useCopy is false the calling code has locked - // hiddenColumns - LOCK.readLock().lock(); - } + LOCK.readLock().lock(); - if (hiddenColumns != null) + if (hiddenColumns != null) + { + int i = 0; + for (i = 0; i < hiddenColumns.size() + && (current <= hiddenColumns.get(i)[0]); ++i) { - int i = 0; - for (i = 0; i < hiddenColumns.size() - && (current <= hiddenColumns.get(i)[0]); ++i) + if (current >= hiddenColumns.get(i)[0] + && current <= hiddenColumns.get(i)[1]) { - if (current >= hiddenColumns.get(i)[0] - && current <= hiddenColumns.get(i)[1]) - { - // current is hidden, move to right - current = hiddenColumns.get(i)[1] + 1; - next = current; - nexthiddenregion = i + 1; - } - } - - for (i = hiddenColumns.size() - 1; i >= 0 - && (last >= hiddenColumns.get(i)[1]); --i) - { - if (last >= hiddenColumns.get(i)[0] - && last <= hiddenColumns.get(i)[1]) - { - // last is hidden, move to left - last = hiddenColumns.get(i)[0] - 1; - } + // current is hidden, move to right + current = hiddenColumns.get(i)[1] + 1; + next = current; + nexthiddenregion = i + 1; } + } - // make a local copy of the bit we need - i = nexthiddenregion; - while (i < hiddenColumns.size() - && hiddenColumns.get(i)[0] <= last) + for (i = hiddenColumns.size() - 1; i >= 0 + && (last >= hiddenColumns.get(i)[1]); --i) + { + if (last >= hiddenColumns.get(i)[0] + && last <= hiddenColumns.get(i)[1]) { - int[] region = new int[] { hiddenColumns.get(i)[0], - hiddenColumns.get(i)[1] }; - localHidden.add(region); - i++; + // last is hidden, move to left + last = hiddenColumns.get(i)[0] - 1; } } - } finally - { - if (useCopy) + + // make a local copy of the bit we need + i = nexthiddenregion; + while (i < hiddenColumns.size() && hiddenColumns.get(i)[0] <= last) { - LOCK.readLock().unlock(); + int[] region = new int[] { hiddenColumns.get(i)[0], + hiddenColumns.get(i)[1] }; + localHidden.add(region); + i++; } } + + LOCK.readLock().unlock(); } @Override @@ -1818,91 +1926,6 @@ public class HiddenColumns } /** - * An iterator which iterates over visible regions in a range. - */ - private class VisibleBlocksIterator implements Iterator - { - private List vcontigs = new ArrayList<>(); - - private int currentPosition = 0; - - VisibleBlocksIterator(int start, int end, boolean usecopy) - { - try - { - if (usecopy) - { - LOCK.readLock().lock(); - } - - if (hiddenColumns != null && hiddenColumns.size() > 0) - { - int blockStart = start; - int blockEnd = end; - - // iterate until a region overlaps with [start,end] - int i = 0; - while ((i < hiddenColumns.size()) - && (hiddenColumns.get(i)[1] < start)) - { - i++; - } - - // iterate from start to end, adding each hidden region. Positions are - // absolute, and all regions which *overlap* [start,end] are used. - while (i < hiddenColumns.size() - && (hiddenColumns.get(i)[0] <= end)) - { - int[] region = hiddenColumns.get(i); - - blockStart = Math.min(blockStart, region[1] + 1); - blockEnd = Math.min(blockEnd, region[0]); - - int[] contig = new int[] { blockStart, blockEnd }; - vcontigs.add(contig); - - blockStart = region[1] + 1; - blockEnd = end; - - i++; - } - - if (end > blockStart) - { - int[] contig = new int[] { blockStart, end }; - vcontigs.add(contig); - } - } - else - { - int[] contig = new int[] { start, end }; - vcontigs.add(contig); - } - } finally - { - if (usecopy) - { - LOCK.readLock().unlock(); - } - } - } - - @Override - public boolean hasNext() - { - return (currentPosition < vcontigs.size()); - } - - @Override - public int[] next() - { - int[] result = vcontigs.get(currentPosition); - currentPosition++; - return result; - } - } - - /** * An iterator which iterates over visible regions in a range. The range is * specified in terms of visible column positions. Provides a special * "endsAtHidden" indicator to allow callers to determine if the final visible @@ -2036,5 +2059,4 @@ public class HiddenColumns return endsAtHidden; } } - } -- 1.7.10.2