From b913f577205e8df5cdb6342ef161d7fd40ff15a4 Mon Sep 17 00:00:00 2001 From: kiramt Date: Mon, 29 Jan 2018 10:21:58 +0000 Subject: [PATCH 1/1] JAL-2759 make a new cursor instead of resetting, updated after review --- src/jalview/datamodel/HiddenColumns.java | 13 +++--- src/jalview/datamodel/HiddenColumnsCursor.java | 49 +++++++------------- .../jalview/datamodel/HiddenColumnsCursorTest.java | 9 ++-- 3 files changed, 31 insertions(+), 40 deletions(-) diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index 8cb7971..321a606 100644 --- 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 --- 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 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 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 hiddenCols) + protected HiddenColumnsCursor(List hiddenCols) { resetCursor(hiddenCols, 0, 0); } + protected HiddenColumnsCursor(List 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 hiddenCols, int index, + private void resetCursor(List 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 --- a/test/jalview/datamodel/HiddenColumnsCursorTest.java +++ b/test/jalview/datamodel/HiddenColumnsCursorTest.java @@ -44,7 +44,8 @@ public class HiddenColumnsCursorTest List 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 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); -- 1.7.10.2