JAL-2674 Tidies
authorkiramt <k.mourao@dundee.ac.uk>
Fri, 29 Sep 2017 14:01:08 +0000 (15:01 +0100)
committerkiramt <k.mourao@dundee.ac.uk>
Fri, 29 Sep 2017 14:01:08 +0000 (15:01 +0100)
benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java
src/jalview/datamodel/HiddenColumns.java
test/jalview/datamodel/HiddenColumnsTest.java

index e97baba..1710c85 100644 (file)
@@ -137,20 +137,4 @@ public class HiddenColsIteratorsBenchmark {
         }
         return blockStart;
        }
-       
-       @Benchmark
-       @BenchmarkMode({Mode.Throughput})
-       public int benchLoop1(HiddenColsAndStartState tstate)
-       {
-               int res = 0;
-               int startx = tstate.visibleColumn;
-               List<Integer> positions = tstate.h.findHiddenRegionPositions(startx, startx+60);
-               
-           for (int pos : positions)
-        {
-          res = pos - startx;
-          Blackhole.consumeCPU(5);
-        }
-           return res;
-       }
 }
index 5554be0..aa34461 100644 (file)
@@ -24,7 +24,6 @@ import jalview.util.Comparison;
 
 import java.util.ArrayList;
 import java.util.BitSet;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
@@ -117,19 +116,6 @@ public class HiddenColumns
   }
 
   /**
-   * This method is used to return all the HiddenColumn regions and is intended
-   * to remain private. External callers which need a copy of the regions can
-   * call getHiddenColumnsCopyAsList.
-   * 
-   * @return empty list or List of hidden column intervals
-   */
-  private List<int[]> getHiddenRegions()
-  {
-    return hiddenColumns == null ? Collections.<int[]> emptyList()
-            : hiddenColumns;
-  }
-
-  /**
    * Output regions data as a string. String is in the format:
    * reg0[0]<between>reg0[1]<delimiter>reg1[0]<between>reg1[1] ... regn[1]
    * 
@@ -405,61 +391,7 @@ public class HiddenColumns
 
   }
 
-  /**
-   * Use this method to determine the set of hiddenRegion start positions
-   * between absolute position <start> and absolute position <end>
-   * 
-   * @param start
-   *          absolute residue to start from
-   * @param end
-   *          absolute residue to end at
-   * 
-   * @return list of column numbers in *visible* view where hidden regions start
-   */
-  public List<Integer> findHiddenRegionPositions(int start, int end)
-  {
-    try
-    {
-      LOCK.readLock().lock();
-      List<Integer> positions = null;
-
-      if (hiddenColumns != null)
-      {
-        positions = new ArrayList<>(hiddenColumns.size());
 
-        // navigate to start, keeping count of hidden columns
-        int i = 0;
-        int hiddenSoFar = 0;
-        while ((i < hiddenColumns.size())
-                && (hiddenColumns.get(i)[0] < start))
-        {
-          int[] region = hiddenColumns.get(i);
-          hiddenSoFar += region[1] - region[0] + 1;
-          i++;
-        }
-
-        // iterate from start to end, adding start positions of each
-        // hidden region. Positions are visible columns count, not absolute
-        while (i < hiddenColumns.size()
-                && (hiddenColumns.get(i)[0] < end))
-        {
-          int[] region = hiddenColumns.get(i);
-          positions.add(region[0] - hiddenSoFar);
-          hiddenSoFar += region[1] - region[0] + 1;
-          i++;
-        }
-      }
-      else
-      {
-        positions = new ArrayList<>();
-      }
-
-      return positions;
-    } finally
-    {
-      LOCK.readLock().unlock();
-    }
-  }
 
   /**
    * This method returns the rightmost limit of a region of an alignment with
@@ -796,9 +728,8 @@ public class HiddenColumns
 
       // Simply walk along the sequence whilst watching for hidden column
       // boundaries
-      List<int[]> regions = getHiddenRegions();
+      Iterator<int[]> regions = iterator();
       int spos = fpos;
-      int rcount = 0;
       int hideStart = seq.getLength();
       int hideEnd = -1;
       int visPrev = 0;
@@ -819,9 +750,9 @@ public class HiddenColumns
           }
           lastP = p;
           // update hidden region start/end
-          while (hideEnd < p && rcount < regions.size())
+          while (hideEnd < p && regions.hasNext())
           {
-            int[] region = regions.get(rcount++);
+            int[] region = regions.next();
             visPrev = visNext;
             visNext += region[0] - visPrev;
             hideStart = region[0];
@@ -866,7 +797,8 @@ public class HiddenColumns
    */
   public void makeVisibleAnnotation(AlignmentAnnotation alignmentAnnotation)
   {
-    makeVisibleAnnotation(-1, -1, alignmentAnnotation);
+    makeVisibleAnnotation(0, alignmentAnnotation.annotations.length,
+            alignmentAnnotation);
   }
 
   /**
@@ -892,11 +824,6 @@ public class HiddenColumns
 
       if (alignmentAnnotation.annotations != null)
       {
-        if (start == end && end == -1)
-        {
-          startFrom = 0;
-          endAt = alignmentAnnotation.annotations.length;
-        }
         if (hiddenColumns != null && hiddenColumns.size() > 0)
         {
           removeHiddenAnnotation(startFrom, endAt, alignmentAnnotation);
@@ -1129,60 +1056,69 @@ public class HiddenColumns
   private void propagateInsertions(SequenceI profileseq, AlignmentI al,
           SequenceI origseq)
   {
-    char gc = al.getGapCharacter();
-
-    // take the set of hidden columns, and the set of gaps in origseq,
-    // and remove all the hidden gaps from hiddenColumns
-
-    // first get the gaps as a Bitset
-    BitSet gaps = origseq.gapBitset();
-
-    // now calculate hidden ^ not(gap)
-    BitSet hidden = new BitSet();
-    markHiddenRegions(hidden);
-    hidden.andNot(gaps);
-    hiddenColumns = null;
-    this.hideMarkedBits(hidden);
-
-    // for each sequence in the alignment, except the profile sequence,
-    // insert gaps corresponding to each hidden region
-    // but where each hidden column region is shifted backwards by the number of
-    // preceding visible gaps
-    // update hidden columns at the same time
-    Iterator<int[]> regions = iterator();
-    ArrayList<int[]> newhidden = new ArrayList<>();
-
-    int numGapsBefore = 0;
-    int gapPosition = 0;
-    while (regions.hasNext())
+    try
     {
-      // get region coordinates accounting for gaps
-      // we can rely on gaps not being *in* hidden regions because we already
-      // removed those
-      int[] region = regions.next();
-      while (gapPosition < region[0])
-      {
-        gapPosition++;
-        if (gaps.get(gapPosition))
+      LOCK.writeLock().lock();
+
+      char gc = al.getGapCharacter();
+
+      // take the set of hidden columns, and the set of gaps in origseq,
+      // and remove all the hidden gaps from hiddenColumns
+
+      // first get the gaps as a Bitset
+      BitSet gaps = origseq.gapBitset();
+
+      // now calculate hidden ^ not(gap)
+      BitSet hidden = new BitSet();
+      markHiddenRegions(hidden);
+      hidden.andNot(gaps);
+      hiddenColumns = null;
+      this.hideMarkedBits(hidden);
+
+      // for each sequence in the alignment, except the profile sequence,
+      // insert gaps corresponding to each hidden region
+      // but where each hidden column region is shifted backwards by the number
+      // of
+      // preceding visible gaps
+      // update hidden columns at the same time
+      Iterator<int[]> regions = iterator();
+      ArrayList<int[]> newhidden = new ArrayList<>();
+
+      int numGapsBefore = 0;
+      int gapPosition = 0;
+      while (regions.hasNext())
+      {
+        // get region coordinates accounting for gaps
+        // we can rely on gaps not being *in* hidden regions because we already
+        // removed those
+        int[] region = regions.next();
+        while (gapPosition < region[0])
         {
-          numGapsBefore++;
+          gapPosition++;
+          if (gaps.get(gapPosition))
+          {
+            numGapsBefore++;
+          }
         }
-      }
 
-      int left = region[0] - numGapsBefore;
-      int right = region[1] - numGapsBefore;
-      newhidden.add(new int[] { left, right });
+        int left = region[0] - numGapsBefore;
+        int right = region[1] - numGapsBefore;
+        newhidden.add(new int[] { left, right });
 
-      // make a string with number of gaps = length of hidden region
-      StringBuffer sb = new StringBuffer();
-      for (int s = 0; s < right - left + 1; s++)
-      {
-        sb.append(gc);
-      }
-      padGaps(sb, left, profileseq, al);
+        // make a string with number of gaps = length of hidden region
+        StringBuffer sb = new StringBuffer();
+        for (int s = 0; s < right - left + 1; s++)
+        {
+          sb.append(gc);
+        }
+        padGaps(sb, left, profileseq, al);
 
+      }
+      hiddenColumns = newhidden;
+    } finally
+    {
+      LOCK.writeLock().unlock();
     }
-    hiddenColumns = newhidden;
   }
 
   /**
@@ -1712,7 +1648,8 @@ public class HiddenColumns
         if (hiddenColumns != null)
         {
           int i = 0;
-          for (i = 0; i < hiddenColumns.size(); ++i)
+          for (i = 0; i < hiddenColumns.size()
+                  && (current < hiddenColumns.get(i)[0]); ++i)
           {
             if (current >= hiddenColumns.get(i)[0]
                     && current <= hiddenColumns.get(i)[1])
@@ -1721,15 +1658,12 @@ public class HiddenColumns
               current = hiddenColumns.get(i)[1] + 1;
               next = current;
             }
-            if (current < hiddenColumns.get(i)[0])
-            {
-              break;
-            }
           }
 
           lasthiddenregion = i - 1;
 
-          for (i = hiddenColumns.size() - 1; i >= 0; --i)
+          for (i = hiddenColumns.size() - 1; i >= 0
+                  && (last > hiddenColumns.get(i)[1]); --i)
           {
             if (last >= hiddenColumns.get(i)[0]
                     && last <= hiddenColumns.get(i)[1])
@@ -1737,10 +1671,6 @@ public class HiddenColumns
               // last is hidden, move to left
               last = hiddenColumns.get(i)[0] - 1;
             }
-            if (last > hiddenColumns.get(i)[1])
-            {
-              break;
-            }
           }
 
           // make a local copy of the bit we need
index a3d4623..192a822 100644 (file)
@@ -32,7 +32,6 @@ import jalview.util.Comparison;
 import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Random;
 
 import org.testng.annotations.BeforeClass;
@@ -843,49 +842,6 @@ public class HiddenColumnsTest
   }
 
   @Test(groups = { "Functional" })
-  public void testFindHiddenRegionPositions()
-  {
-    HiddenColumns hc = new HiddenColumns();
-
-    List<Integer> positions = hc.findHiddenRegionPositions(0, 20);
-    assertTrue(positions.isEmpty());
-
-    hc.hideColumns(3, 7);
-    hc.hideColumns(10, 10);
-    hc.hideColumns(14, 15);
-
-    positions = hc.findHiddenRegionPositions(0, 20);
-    assertEquals(3, positions.size());
-    assertEquals(3, positions.get(0).intValue());
-    assertEquals(5, positions.get(1).intValue());
-    assertEquals(8, positions.get(2).intValue());
-
-    positions = hc.findHiddenRegionPositions(7, 20);
-    assertEquals(2, positions.size());
-    assertEquals(5, positions.get(0).intValue());
-    assertEquals(8, positions.get(1).intValue());
-
-    positions = hc.findHiddenRegionPositions(11, 13);
-    assertEquals(0, positions.size());
-
-    positions = hc.findHiddenRegionPositions(7, 20);
-    assertEquals(2, positions.size());
-    assertEquals(5, positions.get(0).intValue());
-    assertEquals(8, positions.get(1).intValue());
-
-    positions = hc.findHiddenRegionPositions(0, 1);
-    assertEquals(0, positions.size());
-
-    positions = hc.findHiddenRegionPositions(17, 20);
-    assertEquals(0, positions.size());
-
-    positions = hc.findHiddenRegionPositions(10, 15);
-    assertEquals(2, positions.size());
-    assertEquals(5, positions.get(0).intValue());
-    assertEquals(8, positions.get(1).intValue());
-  }
-
-  @Test(groups = { "Functional" })
   public void testRegionsToString()
   {
     HiddenColumns hc = new HiddenColumns();