From: kiramt <k.mourao@dundee.ac.uk>
Date: Wed, 22 Nov 2017 14:43:23 +0000 (+0000)
Subject: JAL-2759 performance improvements to hideColumns
X-Git-Tag: Release_2_10_4~55^2~1^2~55
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=4d3cf3e91660d6b1c5bcdee4bef34c2747ac7023;p=jalview.git

JAL-2759 performance improvements to hideColumns
---

diff --git a/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java b/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java
index 6e4742a..ae2d628 100644
--- a/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java
+++ b/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java
@@ -98,7 +98,7 @@ public class HiddenColumnsBenchmark
 	
 	/* Convention: functions in alphabetical order */
 	
-	@Benchmark
+/*	@Benchmark
 	@BenchmarkMode({Mode.Throughput})
 	public int benchAdjustForHiddenColumns(HiddenColsAndStartState tstate)
 	{
@@ -118,7 +118,7 @@ public class HiddenColumnsBenchmark
     {
 		return tstate.h.getSize();
     }
-
+*/
    @Benchmark
     @BenchmarkMode({Mode.Throughput})
     public HiddenColumns benchHideCols(HiddenColsAndStartState tstate) 
@@ -127,7 +127,7 @@ public class HiddenColumnsBenchmark
     			tstate.visibleColumn+2000); 
     	return tstate.h;
     }
-   
+   /*
 	@Benchmark
 	@BenchmarkMode({Mode.Throughput})
     public boolean benchIsVisible(HiddenColsAndStartState tstate) 
@@ -152,6 +152,6 @@ public class HiddenColumnsBenchmark
     	tstate.h.revealAllHiddenColumns(sel);
     	return tstate.h;
     }
-	
+	*/
 	
 }
\ No newline at end of file
diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java
index fa5d0a0..3b1a574 100644
--- a/src/jalview/datamodel/HiddenColumns.java
+++ b/src/jalview/datamodel/HiddenColumns.java
@@ -188,10 +188,28 @@ public class HiddenColumns
         wasAlreadyLocked = true;
       }
 
+      int previndex = 0;
+      int prevHiddenCount = 0;
+      int regionindex = 0;
       if (hiddenColumns == null)
       {
         hiddenColumns = new ArrayList<>();
       }
+      else
+      {
+        // set up cursor reset values
+        HiddenCursorPosition cursorPos = cursor.findRegionForColumn(start);
+        regionindex = cursorPos.getRegionIndex();
+
+        if (regionindex > 0)
+        {
+          // get previous index and hidden count for updating the cursor later
+          previndex = regionindex - 1;
+          int[] prevRegion = hiddenColumns.get(previndex);
+          prevHiddenCount = cursorPos.getHiddenSoFar()
+                  - (prevRegion[1] - prevRegion[0] + 1);
+        }
+      }
 
       /*
        * new range follows everything else; check first to avoid looping over whole hiddenColumns collection
@@ -208,18 +226,23 @@ public class HiddenColumns
          * appropriate
          */
         boolean added = false;
-        for (int i = 0; !added && i < hiddenColumns.size(); i++)
+        if (regionindex > 0)
+        {
+          added = insertRangeAtRegion(regionindex - 1, start, end);
+        }
+        if (!added && regionindex < hiddenColumns.size())
         {
-          added = insertRangeAtRegion(i, start, end);
-        } // for
+          insertRangeAtRegion(regionindex, start, end);
+        }
       }
-      if (!wasAlreadyLocked)
-      {
-        cursor.resetCursor(hiddenColumns);
 
-        // reset the number of columns so they will be recounted
-        numColumns = 0;
-      }
+      // reset the cursor to just before our insertion point: this saves
+      // a lot of reprocessing in large alignments
+      cursor.resetCursor(hiddenColumns, previndex, prevHiddenCount);
+
+      // reset the number of columns so they will be recounted
+      numColumns = 0;
+
     } finally
     {
       if (!wasAlreadyLocked)
@@ -287,6 +310,9 @@ public class HiddenColumns
           break;
         }
         region[1] = Math.max(nextRegion[1], end);
+
+        // in theory this is faster than hiddenColumns.remove(i+1)
+        // benchmarking results a bit ambivalent
         hiddenColumns.subList(i + 1, i + 2).clear();
       }
       added = true;
diff --git a/src/jalview/datamodel/HiddenColumnsCursor.java b/src/jalview/datamodel/HiddenColumnsCursor.java
index 04d7189..9cb3298 100644
--- a/src/jalview/datamodel/HiddenColumnsCursor.java
+++ b/src/jalview/datamodel/HiddenColumnsCursor.java
@@ -48,15 +48,34 @@ public class HiddenColumnsCursor
    * already exist.
    * 
    * @param hiddenCols
+   *          new hidden columns collection
    */
   protected void resetCursor(List<int[]> hiddenCols)
   {
+    resetCursor(hiddenCols, 0, 0);
+  }
+
+  /**
+   * Reset the cursor with a new hidden columns collection, where we know in
+   * advance the index and hidden columns count of a particular location.
+   * 
+   * @param hiddenCols
+   *          new hidden columns collection
+   * @param index
+   *          cursor index to reset to
+   * @param hiddencount
+   *          hidden columns count to reset to
+   */
+  protected void resetCursor(List<int[]> hiddenCols, int index,
+          int hiddencount)
+  {
     hiddenColumns = hiddenCols;
     if ((hiddenCols != null) && (!hiddenCols.isEmpty()))
     {
       firstColumn = hiddenColumns.get(0)[0];
       HiddenCursorPosition oldpos = cursorPos.get();
-      HiddenCursorPosition newpos = new HiddenCursorPosition(0, 0);
+      HiddenCursorPosition newpos = new HiddenCursorPosition(index,
+              hiddencount);
       cursorPos.compareAndSet(oldpos, newpos);
     }
   }
diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java
index e3dd1cd..5d57ed2 100644
--- a/test/jalview/datamodel/HiddenColumnsTest.java
+++ b/test/jalview/datamodel/HiddenColumnsTest.java
@@ -511,6 +511,16 @@ public class HiddenColumnsTest
     regions = cs.iterator();
     assertEquals(1, cs.getNumberOfRegions());
     assertEquals("[1, 7]", Arrays.toString(regions.next()));
+
+    cs.revealAllHiddenColumns(colsel);
+    cs.hideColumns(15, 18);
+    cs.hideColumns(2, 4);
+    cs.hideColumns(7, 9);
+    regions = cs.iterator();
+    assertEquals(3, cs.getNumberOfRegions());
+    assertEquals("[2, 4]", Arrays.toString(regions.next()));
+    assertEquals("[7, 9]", Arrays.toString(regions.next()));
+    assertEquals("[15, 18]", Arrays.toString(regions.next()));
   }
 
   /**