JAL-2759 make a new cursor instead of resetting, updated after review

diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java
index 8cb7971..321a606 100644 (file)
--- a/src/jalview/datamodel/HiddenColumns.java
+++ b/src/jalview/datamodel/HiddenColumns.java
@@ -140,7 +140,7 @@ public class HiddenColumns
             numColumns += region[1] - region[0] + 1;
           }
         }
-        cursor.resetCursor(hiddenColumns);
+        cursor = new HiddenColumnsCursor(hiddenColumns);
       }
     } finally
     {
@@ -208,7 +208,8 @@ public class HiddenColumns
 
       // reset the cursor to just before our insertion point: this saves
       // a lot of reprocessing in large alignments
-      cursor.resetCursor(hiddenColumns, previndex, prevHiddenCount);
+      cursor = new HiddenColumnsCursor(hiddenColumns, previndex,
+              prevHiddenCount);
 
       // reset the number of columns so they will be recounted
       numColumns = 0;
@@ -301,7 +302,7 @@ public class HiddenColumns
       {
         hideColumns(r[0], r[1]);
       }
-      cursor.resetCursor(hiddenColumns);
+      cursor = new HiddenColumnsCursor(hiddenColumns);
       numColumns = 0;
     } finally
     {
@@ -326,7 +327,7 @@ public class HiddenColumns
         }
       }
       hiddenColumns.clear();
-      cursor.resetCursor(hiddenColumns);
+      cursor = new HiddenColumnsCursor(hiddenColumns);
       numColumns = 0;
 
     } finally
@@ -782,7 +783,7 @@ public class HiddenColumns
         lastSet = inserts.nextClearBit(firstSet);
         hideColumns(firstSet, lastSet - 1);
       }
-      cursor.resetCursor(hiddenColumns);
+      cursor = new HiddenColumnsCursor(hiddenColumns);
       numColumns = 0;
     } finally
     {
@@ -866,7 +867,7 @@ public class HiddenColumns
         }
 
         hiddenColumns.subList(startindex, endindex + 1).clear();
-        cursor.resetCursor(hiddenColumns);
+        cursor = new HiddenColumnsCursor(hiddenColumns);
         numColumns = 0;
       }
     } finally

diff --git a/src/jalview/datamodel/HiddenColumnsCursor.java b/src/jalview/datamodel/HiddenColumnsCursor.java
index a875f87..9b3854d 100644 (file)
--- a/src/jalview/datamodel/HiddenColumnsCursor.java
+++ b/src/jalview/datamodel/HiddenColumnsCursor.java
@@ -22,7 +22,6 @@ package jalview.datamodel;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicReference;
 
 public class HiddenColumnsCursor
 {
@@ -31,31 +30,24 @@ public class HiddenColumnsCursor
 
   private List<int[]> hiddenColumns = new ArrayList<>();
 
-  // AtomicReference to hold the current region index and hidden column count
-  // Could be done with synchronisation but benchmarking shows this way is 2x
-  // faster
-  private final AtomicReference<HiddenCursorPosition> cursorPos = new AtomicReference<>(
-          new HiddenCursorPosition(0, 0));
+  private HiddenCursorPosition cursorPos = new HiddenCursorPosition(0, 0);
 
   protected HiddenColumnsCursor()
   {
 
   }
 
-  /**
-   * Reset the cursor with a new hidden columns collection. Calls to resetCursor
-   * should be made from within a writeLock in the HiddenColumns class - since
-   * changes to the hiddenColumns collection require a writeLock the lock should
-   * already exist.
-   * 
-   * @param hiddenCols
-   *          new hidden columns collection
-   */
-  protected void resetCursor(List<int[]> hiddenCols)
+  protected HiddenColumnsCursor(List<int[]> hiddenCols)
   {
     resetCursor(hiddenCols, 0, 0);
   }
 
+  protected HiddenColumnsCursor(List<int[]> hiddenCols, int index,
+          int hiddencount)
+  {
+    resetCursor(hiddenCols, index, hiddencount);
+  }
+
   /**
    * Reset the cursor with a new hidden columns collection, where we know in
    * advance the index and hidden columns count of a particular location.
@@ -67,17 +59,15 @@ public class HiddenColumnsCursor
    * @param hiddencount
    *          hidden columns count to reset to
    */
-  protected void resetCursor(List<int[]> hiddenCols, int index,
+  private void resetCursor(List<int[]> hiddenCols, int index,
           int hiddencount)
   {
     hiddenColumns = hiddenCols;
     if (!hiddenCols.isEmpty())
     {
       firstColumn = hiddenColumns.get(0)[0];
-      HiddenCursorPosition oldpos = cursorPos.get();
-      HiddenCursorPosition newpos = new HiddenCursorPosition(index,
+      cursorPos = new HiddenCursorPosition(index,
               hiddencount);
-      cursorPos.compareAndSet(oldpos, newpos);
     }
   }
 
@@ -100,7 +90,7 @@ public class HiddenColumnsCursor
       // nothing changes; otherwise
       // we deleted the last region (index=hiddenCols.size()-1)
       // or the index was at the end of the alignment (index=hiddenCols.size())
-      HiddenCursorPosition oldpos = cursorPos.get();
+      HiddenCursorPosition oldpos = cursorPos; // .get();
 
       int index = oldpos.getRegionIndex();
       if (index >= hiddenColumns.size() - 1)
@@ -108,9 +98,8 @@ public class HiddenColumnsCursor
         // deleted last region, index is now end of alignment
         index = hiddenCols.size();
 
-        HiddenCursorPosition newpos = new HiddenCursorPosition(index,
+        cursorPos = new HiddenCursorPosition(index,
                 oldpos.getHiddenSoFar());
-        cursorPos.compareAndSet(oldpos, newpos);
       }
     }
     hiddenColumns = hiddenCols;
@@ -136,7 +125,7 @@ public class HiddenColumnsCursor
       return null;
     }
 
-    HiddenCursorPosition oldpos = cursorPos.get();
+    HiddenCursorPosition oldpos = cursorPos;// .get();
     int index = oldpos.getRegionIndex();
     int hiddenCount = oldpos.getHiddenSoFar();
 
@@ -185,10 +174,9 @@ public class HiddenColumnsCursor
     if (index != oldpos.getRegionIndex()
             || hiddenCount != oldpos.getHiddenSoFar())
     {
-      HiddenCursorPosition newpos = new HiddenCursorPosition(index,
+      cursorPos = new HiddenCursorPosition(index,
               hiddenCount);
-      cursorPos.compareAndSet(oldpos, newpos);
-      return newpos;
+      return cursorPos;
     }
     return oldpos;
   }
@@ -208,7 +196,7 @@ public class HiddenColumnsCursor
       return null;
     }
 
-    HiddenCursorPosition oldpos = cursorPos.get();
+    HiddenCursorPosition oldpos = cursorPos;// .get();
     int index = oldpos.getRegionIndex();
     int hiddenCount = oldpos.getHiddenSoFar();
 
@@ -245,10 +233,9 @@ public class HiddenColumnsCursor
     if (index != oldpos.getRegionIndex()
             || hiddenCount != oldpos.getHiddenSoFar())
     {
-      HiddenCursorPosition newpos = new HiddenCursorPosition(index,
+      cursorPos = new HiddenCursorPosition(index,
               hiddenCount);
-      cursorPos.compareAndSet(oldpos, newpos);
-      return newpos;
+      return cursorPos;
     }
     return oldpos;
   }

diff --git a/test/jalview/datamodel/HiddenColumnsCursorTest.java b/test/jalview/datamodel/HiddenColumnsCursorTest.java
index 257ea95..0deed08 100644 (file)
--- a/test/jalview/datamodel/HiddenColumnsCursorTest.java
+++ b/test/jalview/datamodel/HiddenColumnsCursorTest.java
@@ -44,7 +44,8 @@ public class HiddenColumnsCursorTest
     List<int[]> hidden = new ArrayList<>();
     hidden.add(new int[] { 53, 76 });
     hidden.add(new int[] { 104, 125 });
-    cursor.resetCursor(hidden);
+
+    cursor = new HiddenColumnsCursor(hidden);
 
     int regionIndex = cursor.findRegionForColumn(126).getRegionIndex();
     assertEquals(2, regionIndex);
@@ -77,7 +78,8 @@ public class HiddenColumnsCursorTest
     assertEquals(0, regionIndex);
 
     hidden.add(new int[] { 138, 155 });
-    cursor.resetCursor(hidden);
+
+    cursor = new HiddenColumnsCursor(hidden);
 
     regionIndex = cursor.findRegionForColumn(160).getRegionIndex();
     assertEquals(3, regionIndex);
@@ -100,7 +102,8 @@ public class HiddenColumnsCursorTest
     List<int[]> hidden = new ArrayList<>();
     hidden.add(new int[] { 53, 76 });
     hidden.add(new int[] { 104, 125 });
-    cursor.resetCursor(hidden);
+
+    cursor = new HiddenColumnsCursor(hidden);
 
     int offset = cursor.findRegionForVisColumn(80).getHiddenSoFar();
     assertEquals(46, offset);