Bug 509685 - Performance leak with huge amount of columns

Change-Id: I41c1bdda8c9297808e3a1040e5ecaf91ad1d0d95
Signed-off-by: Dirk Fauth <dirk.fauth@googlemail.com>

4 files changed, 322 insertions, 110 deletions

diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/AbstractColumnHideShowLayer.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/AbstractColumnHideShowLayer.java
index 26406f89..7da358f3 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/AbstractColumnHideShowLayer.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/AbstractColumnHideShowLayer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2013 Original authors and others.
+ * Copyright (c) 2012, 2016 Original authors and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -26,10 +26,10 @@ import org.eclipse.nebula.widgets.nattable.layer.LayerUtil;
 import org.eclipse.nebula.widgets.nattable.layer.event.ILayerEvent;
 import org.eclipse.nebula.widgets.nattable.layer.event.IStructuralChangeEvent;
 
-public abstract class AbstractColumnHideShowLayer extends
-        AbstractLayerTransform implements IUniqueIndexLayer {
+public abstract class AbstractColumnHideShowLayer extends AbstractLayerTransform implements IUniqueIndexLayer {
 
     private List<Integer> cachedVisibleColumnIndexOrder;
+    private Map<Integer, Integer> cachedVisibleColumnIndexPositionMap;
 
     private Map<Integer, Integer> cachedHiddenColumnIndexToPositionMap;
 
@@ -65,23 +65,21 @@ public abstract class AbstractColumnHideShowLayer extends
             return -1;
         }
 
-        Integer columnIndex = getCachedVisibleColumnIndexes().get(
-                columnPosition);
-        if (columnIndex != null) {
-            return columnIndex.intValue();
-        } else {
-            return -1;
-        }
+        Integer columnIndex = getCachedVisibleColumnIndexes().get(columnPosition);
+        return (columnIndex != null) ? columnIndex : -1;
     }
 
     @Override
     public int getColumnPositionByIndex(int columnIndex) {
-        return getCachedVisibleColumnIndexes().indexOf(
-                Integer.valueOf(columnIndex));
+        if (this.cachedVisibleColumnIndexPositionMap == null) {
+            cacheVisibleColumnIndexes();
+        }
+
+        Integer position = this.cachedVisibleColumnIndexPositionMap.get(columnIndex);
+        return (position != null) ? position : -1;
     }
 
-    public Collection<Integer> getColumnPositionsByIndexes(
-            Collection<Integer> columnIndexes) {
+    public Collection<Integer> getColumnPositionsByIndexes(Collection<Integer> columnIndexes) {
         Collection<Integer> columnPositions = new HashSet<Integer>();
         for (int columnIndex : columnIndexes) {
             columnPositions.add(getColumnPositionByIndex(columnIndex));
@@ -92,26 +90,18 @@ public abstract class AbstractColumnHideShowLayer extends
     @Override
     public int localToUnderlyingColumnPosition(int localColumnPosition) {
         int columnIndex = getColumnIndexByPosition(localColumnPosition);
-        return ((IUniqueIndexLayer) getUnderlyingLayer())
-                .getColumnPositionByIndex(columnIndex);
+        return ((IUniqueIndexLayer) getUnderlyingLayer()).getColumnPositionByIndex(columnIndex);
     }
 
     @Override
-    public int underlyingToLocalColumnPosition(ILayer sourceUnderlyingLayer,
-            int underlyingColumnPosition) {
-        int columnIndex = getUnderlyingLayer().getColumnIndexByPosition(
-                underlyingColumnPosition);
+    public int underlyingToLocalColumnPosition(ILayer sourceUnderlyingLayer, int underlyingColumnPosition) {
+        int columnIndex = getUnderlyingLayer().getColumnIndexByPosition(underlyingColumnPosition);
         int columnPosition = getColumnPositionByIndex(columnIndex);
         if (columnPosition >= 0) {
             return columnPosition;
         } else {
-            Integer hiddenColumnPosition = this.cachedHiddenColumnIndexToPositionMap
-                    .get(Integer.valueOf(columnIndex));
-            if (hiddenColumnPosition != null) {
-                return hiddenColumnPosition.intValue();
-            } else {
-                return -1;
-            }
+            Integer hiddenColumnPosition = this.cachedHiddenColumnIndexToPositionMap.get(columnIndex);
+            return (hiddenColumnPosition != null) ? hiddenColumnPosition : -1;
         }
     }
 
@@ -123,22 +113,22 @@ public abstract class AbstractColumnHideShowLayer extends
 
         for (Range underlyingColumnPositionRange : underlyingColumnPositionRanges) {
             int startColumnPosition = getAdjustedUnderlyingToLocalStartPosition(
-                    sourceUnderlyingLayer, underlyingColumnPositionRange.start,
+                    sourceUnderlyingLayer,
+                    underlyingColumnPositionRange.start,
                     underlyingColumnPositionRange.end);
             int endColumnPosition = getAdjustedUnderlyingToLocalEndPosition(
-                    sourceUnderlyingLayer, underlyingColumnPositionRange.end,
+                    sourceUnderlyingLayer,
+                    underlyingColumnPositionRange.end,
                     underlyingColumnPositionRange.start);
 
             // teichstaedt: fixes the problem that ranges where added even if
-            // the
-            // corresponding startPosition weren't found in the underlying
-            // layer.
-            // Without that fix a bunch of ranges of kind Range [-1, 180] which
-            // causes strange behaviour in Freeze- and other Layers were
+            // the corresponding startPosition weren't found in the underlying
+            // layer. Without that fix a bunch of ranges of kind Range [-1, 180]
+            // which causes strange behaviour in Freeze- and other Layers were
             // returned.
             if (startColumnPosition > -1) {
-                localColumnPositionRanges.add(new Range(startColumnPosition,
-                        endColumnPosition));
+                localColumnPositionRanges.add(
+                        new Range(startColumnPosition, endColumnPosition));
             }
         }
 
@@ -146,29 +136,41 @@ public abstract class AbstractColumnHideShowLayer extends
     }
 
     private int getAdjustedUnderlyingToLocalStartPosition(
-            ILayer sourceUnderlyingLayer, int startUnderlyingPosition,
+            ILayer sourceUnderlyingLayer,
+            int startUnderlyingPosition,
             int endUnderlyingPosition) {
-        int localStartColumnPosition = underlyingToLocalColumnPosition(
-                sourceUnderlyingLayer, startUnderlyingPosition);
+
+        int localStartColumnPosition =
+                underlyingToLocalColumnPosition(
+                        sourceUnderlyingLayer,
+                        startUnderlyingPosition);
         int offset = 0;
         while (localStartColumnPosition < 0
                 && (startUnderlyingPosition + offset < endUnderlyingPosition)) {
-            localStartColumnPosition = underlyingToLocalColumnPosition(
-                    sourceUnderlyingLayer, startUnderlyingPosition + offset++);
+            localStartColumnPosition =
+                    underlyingToLocalColumnPosition(
+                            sourceUnderlyingLayer,
+                            startUnderlyingPosition + offset++);
         }
         return localStartColumnPosition;
     }
 
     private int getAdjustedUnderlyingToLocalEndPosition(
-            ILayer sourceUnderlyingLayer, int endUnderlyingPosition,
+            ILayer sourceUnderlyingLayer,
+            int endUnderlyingPosition,
             int startUnderlyingPosition) {
-        int localEndColumnPosition = underlyingToLocalColumnPosition(
-                sourceUnderlyingLayer, endUnderlyingPosition - 1);
+
+        int localEndColumnPosition =
+                underlyingToLocalColumnPosition(
+                        sourceUnderlyingLayer,
+                        endUnderlyingPosition - 1);
         int offset = 0;
         while (localEndColumnPosition < 0
                 && (endUnderlyingPosition - offset > startUnderlyingPosition)) {
-            localEndColumnPosition = underlyingToLocalColumnPosition(
-                    sourceUnderlyingLayer, endUnderlyingPosition - offset++);
+            localEndColumnPosition =
+                    underlyingToLocalColumnPosition(
+                            sourceUnderlyingLayer,
+                            endUnderlyingPosition - offset++);
         }
         return localEndColumnPosition + 1;
     }
@@ -191,8 +193,7 @@ public abstract class AbstractColumnHideShowLayer extends
 
     @Override
     public int getStartXOfColumnPosition(int localColumnPosition) {
-        Integer cachedStartX = this.startXCache.get(Integer
-                .valueOf(localColumnPosition));
+        Integer cachedStartX = this.startXCache.get(localColumnPosition);
         if (cachedStartX != null) {
             return cachedStartX.intValue();
         }
@@ -202,23 +203,19 @@ public abstract class AbstractColumnHideShowLayer extends
         if (underlyingPosition < 0) {
             return -1;
         }
-        int underlyingStartX = underlyingLayer
-                .getStartXOfColumnPosition(underlyingPosition);
+        int underlyingStartX = underlyingLayer.getStartXOfColumnPosition(underlyingPosition);
         if (underlyingStartX < 0) {
             return -1;
         }
 
         for (Integer hiddenIndex : getHiddenColumnIndexes()) {
-            int hiddenPosition = underlyingLayer
-                    .getColumnPositionByIndex(hiddenIndex.intValue());
+            int hiddenPosition = underlyingLayer.getColumnPositionByIndex(hiddenIndex);
             if (hiddenPosition <= underlyingPosition) {
-                underlyingStartX -= underlyingLayer
-                        .getColumnWidthByPosition(hiddenPosition);
+                underlyingStartX -= underlyingLayer.getColumnWidthByPosition(hiddenPosition);
             }
         }
 
-        this.startXCache.put(Integer.valueOf(localColumnPosition),
-                Integer.valueOf(underlyingStartX));
+        this.startXCache.put(localColumnPosition, underlyingStartX);
         return underlyingStartX;
     }
 
@@ -228,8 +225,7 @@ public abstract class AbstractColumnHideShowLayer extends
 
     @Override
     public int getRowPositionByIndex(int rowIndex) {
-        return ((IUniqueIndexLayer) getUnderlyingLayer())
-                .getRowPositionByIndex(rowIndex);
+        return ((IUniqueIndexLayer) getUnderlyingLayer()).getRowPositionByIndex(rowIndex);
     }
 
     // Hide/show
@@ -265,6 +261,7 @@ public abstract class AbstractColumnHideShowLayer extends
      */
     protected void invalidateCache() {
         this.cachedVisibleColumnIndexOrder = null;
+        this.cachedVisibleColumnIndexPositionMap = null;
         this.startXCache.clear();
     }
 
@@ -277,23 +274,21 @@ public abstract class AbstractColumnHideShowLayer extends
 
     private void cacheVisibleColumnIndexes() {
         this.cachedVisibleColumnIndexOrder = new ArrayList<Integer>();
+        this.cachedVisibleColumnIndexPositionMap = new HashMap<Integer, Integer>();
         this.cachedHiddenColumnIndexToPositionMap = new HashMap<Integer, Integer>();
         this.startXCache.clear();
 
         ILayer underlyingLayer = getUnderlyingLayer();
         int columnPosition = 0;
-        for (int parentColumnPosition = 0; parentColumnPosition < underlyingLayer
-                .getColumnCount(); parentColumnPosition++) {
-            int columnIndex = underlyingLayer
-                    .getColumnIndexByPosition(parentColumnPosition);
+        for (int parentColumnPosition = 0; parentColumnPosition < underlyingLayer.getColumnCount(); parentColumnPosition++) {
+            int columnIndex = underlyingLayer.getColumnIndexByPosition(parentColumnPosition);
 
             if (!isColumnIndexHidden(columnIndex)) {
-                this.cachedVisibleColumnIndexOrder.add(Integer.valueOf(columnIndex));
+                this.cachedVisibleColumnIndexOrder.add(columnIndex);
+                this.cachedVisibleColumnIndexPositionMap.put(columnIndex, columnPosition);
                 columnPosition++;
             } else {
-                this.cachedHiddenColumnIndexToPositionMap.put(
-                        Integer.valueOf(columnIndex),
-                        Integer.valueOf(columnPosition));
+                this.cachedHiddenColumnIndexToPositionMap.put(columnIndex, columnPosition);
             }
         }
     }
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/ColumnReorderLayer.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/ColumnReorderLayer.java
index 59f2a914..2b5374d7 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/ColumnReorderLayer.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/ColumnReorderLayer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2013 Original authors and others.
+ * Copyright (c) 2012, 2016 Original authors and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -32,40 +32,71 @@ import org.eclipse.nebula.widgets.nattable.layer.event.ILayerEvent;
 import org.eclipse.nebula.widgets.nattable.layer.event.IStructuralChangeEvent;
 import org.eclipse.nebula.widgets.nattable.layer.event.StructuralChangeEventHelper;
 import org.eclipse.nebula.widgets.nattable.layer.event.StructuralDiff;
+import org.eclipse.nebula.widgets.nattable.reorder.action.ColumnReorderDragMode;
 import org.eclipse.nebula.widgets.nattable.reorder.command.ColumnReorderCommandHandler;
 import org.eclipse.nebula.widgets.nattable.reorder.command.ColumnReorderEndCommandHandler;
+import org.eclipse.nebula.widgets.nattable.reorder.command.ColumnReorderStartCommand;
 import org.eclipse.nebula.widgets.nattable.reorder.command.ColumnReorderStartCommandHandler;
 import org.eclipse.nebula.widgets.nattable.reorder.command.MultiColumnReorderCommandHandler;
 import org.eclipse.nebula.widgets.nattable.reorder.config.DefaultColumnReorderLayerConfiguration;
 import org.eclipse.nebula.widgets.nattable.reorder.event.ColumnReorderEvent;
 
 /**
- * Adds functionality for reordering column(s) Also responsible for
- * saving/loading the column order state.
+ * Layer that is used to add the functionality for column reordering.
  *
  * @see DefaultColumnReorderLayerConfiguration
  */
 public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqueIndexLayer {
 
-    private static final Log log = LogFactory.getLog(ColumnReorderLayer.class);
+    private static final Log LOG = LogFactory.getLog(ColumnReorderLayer.class);
 
     public static final String PERSISTENCE_KEY_COLUMN_INDEX_ORDER = ".columnIndexOrder"; //$NON-NLS-1$
 
     private final IUniqueIndexLayer underlyingLayer;
 
-    // Position X in the List contains the index of column at position X
+    /**
+     * The internal cache of the column index order. Used to track the
+     * reordering performed by this layer. Position X in the List contains the
+     * index of column at position X.
+     */
     protected final List<Integer> columnIndexOrder = new ArrayList<Integer>();
 
+    /**
+     * The internal mapping of index to position values. Used for performance
+     * reasons in {@link #getColumnPositionByIndex(int)} because
+     * {@link List#indexOf(Object)} doesn't scale well.
+     *
+     * @since 1.5
+     */
+    protected final Map<Integer, Integer> indexPositionMapping = new HashMap<Integer, Integer>();
+
     private final Map<Integer, Integer> startXCache = new HashMap<Integer, Integer>();
 
     private int reorderFromColumnPosition;
 
+    /**
+     * Creates a {@link ColumnReorderLayer} on top of the given
+     * {@link IUniqueIndexLayer} and adds the
+     * {@link DefaultColumnReorderLayerConfiguration}.
+     *
+     * @param underlyingLayer
+     *            The underlying layer.
+     */
     public ColumnReorderLayer(IUniqueIndexLayer underlyingLayer) {
         this(underlyingLayer, true);
     }
 
-    public ColumnReorderLayer(IUniqueIndexLayer underlyingLayer,
-            boolean useDefaultConfiguration) {
+    /**
+     * Creates a {@link ColumnReorderLayer} on top of the given
+     * {@link IUniqueIndexLayer}.
+     *
+     * @param underlyingLayer
+     *            The underlying layer.
+     * @param useDefaultConfiguration
+     *            <code>true</code> to add the
+     *            {@link DefaultColumnReorderLayerConfiguration}
+     */
+    public ColumnReorderLayer(IUniqueIndexLayer underlyingLayer, boolean useDefaultConfiguration) {
         super(underlyingLayer);
         this.underlyingLayer = underlyingLayer;
 
@@ -86,7 +117,6 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
                 Collection<StructuralDiff> structuralDiffs = structuralChangeEvent.getColumnDiffs();
                 if (structuralDiffs == null) {
                     // Assume everything changed
-                    this.columnIndexOrder.clear();
                     populateIndexOrder();
                 } else {
                     // only react on ADD or DELETE and not on CHANGE
@@ -94,6 +124,8 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
                             structuralDiffs, this.underlyingLayer, this.columnIndexOrder, true);
                     StructuralChangeEventHelper.handleColumnInsert(
                             structuralDiffs, this.underlyingLayer, this.columnIndexOrder, true);
+                    // update index-position mapping
+                    refreshIndexPositionMapping();
                 }
                 invalidateCache();
             }
@@ -142,6 +174,8 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
             if (isRestoredStateValid(newColumnIndexOrder)) {
                 this.columnIndexOrder.clear();
                 this.columnIndexOrder.addAll(newColumnIndexOrder);
+                // refresh index-position mapping
+                refreshIndexPositionMapping();
             }
 
         }
@@ -157,7 +191,7 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
      */
     protected boolean isRestoredStateValid(List<Integer> newColumnIndexOrder) {
         if (newColumnIndexOrder.size() != getColumnCount()) {
-            log.error("Number of persisted columns (" + newColumnIndexOrder.size() + ") " + //$NON-NLS-1$ //$NON-NLS-2$
+            LOG.error("Number of persisted columns (" + newColumnIndexOrder.size() + ") " + //$NON-NLS-1$ //$NON-NLS-2$
                     "is not the same as the number of columns in the data source (" //$NON-NLS-1$
                     + getColumnCount() + ").\n" + //$NON-NLS-1$
                     "Skipping restore of column ordering"); //$NON-NLS-1$
@@ -165,8 +199,8 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
         }
 
         for (Integer index : newColumnIndexOrder) {
-            if (!this.columnIndexOrder.contains(index)) {
-                log.error("Column index: " + index + " being restored, is not a available in the data soure.\n" + //$NON-NLS-1$ //$NON-NLS-2$
+            if (!this.indexPositionMapping.containsKey(index)) {
+                LOG.error("Column index: " + index + " being restored, is not a available in the data soure.\n" + //$NON-NLS-1$ //$NON-NLS-2$
                         "Skipping restore of column ordering"); //$NON-NLS-1$
                 return false;
             }
@@ -176,15 +210,18 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
 
     // Columns
 
+    /**
+     *
+     * @return the internal kept ordering of column indexes.
+     */
     public List<Integer> getColumnIndexOrder() {
         return this.columnIndexOrder;
     }
 
     @Override
     public int getColumnIndexByPosition(int columnPosition) {
-        if (columnPosition >= 0
-                && columnPosition < this.columnIndexOrder.size()) {
-            return this.columnIndexOrder.get(columnPosition).intValue();
+        if (columnPosition >= 0 && columnPosition < this.columnIndexOrder.size()) {
+            return this.columnIndexOrder.get(columnPosition);
         } else {
             return -1;
         }
@@ -192,7 +229,8 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
 
     @Override
     public int getColumnPositionByIndex(int columnIndex) {
-        return this.columnIndexOrder.indexOf(Integer.valueOf(columnIndex));
+        Integer result = this.indexPositionMapping.get(columnIndex);
+        return (result != null) ? result : -1;
     }
 
     @Override
@@ -244,10 +282,29 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
         return aggregateWidth;
     }
 
+    /**
+     * Initialize the internal column index ordering from a clean state, which
+     * means it reflects the ordering from the underlying layer.
+     */
     private void populateIndexOrder() {
+        this.columnIndexOrder.clear();
         ILayer underlyingLayer = getUnderlyingLayer();
         for (int columnPosition = 0; columnPosition < underlyingLayer.getColumnCount(); columnPosition++) {
-            this.columnIndexOrder.add(Integer.valueOf(underlyingLayer.getColumnIndexByPosition(columnPosition)));
+            int index = underlyingLayer.getColumnIndexByPosition(columnPosition);
+            this.columnIndexOrder.add(index);
+            this.indexPositionMapping.put(index, columnPosition);
+        }
+    }
+
+    /**
+     * Initializes the internal index-position-mapping to reflect the internal
+     * column-index-order.
+     */
+    private void refreshIndexPositionMapping() {
+        this.indexPositionMapping.clear();
+        for (int position = 0; position < this.columnIndexOrder.size(); position++) {
+            int index = this.columnIndexOrder.get(position);
+            this.indexPositionMapping.put(index, position);
         }
     }
 
@@ -261,12 +318,17 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
     }
 
     /**
-     * Moves the column to the <i>LEFT</i> of the toColumnPosition
+     * Moves the given from-column to the specified edge of the column to move
+     * to.
      *
      * @param fromColumnPosition
      *            column position to move
      * @param toColumnPosition
      *            position to move the column to
+     * @param reorderToLeftEdge
+     *            <code>true</code> if the column should be moved to the left of
+     *            the given column to move to, <code>false</code> if it should
+     *            be positioned to the right
      */
     private void moveColumn(int fromColumnPosition, int toColumnPosition, boolean reorderToLeftEdge) {
         if (!reorderToLeftEdge) {
@@ -275,11 +337,23 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
 
         Integer fromColumnIndex = this.columnIndexOrder.get(fromColumnPosition);
         this.columnIndexOrder.add(toColumnPosition, fromColumnIndex);
-
         this.columnIndexOrder.remove(fromColumnPosition + (fromColumnPosition > toColumnPosition ? 1 : 0));
+
+        // update index-position mapping
+        refreshIndexPositionMapping();
+
         invalidateCache();
     }
 
+    /**
+     * Moves the given from-column to the <b>left</b> edge of the column to move
+     * to.
+     *
+     * @param fromColumnPosition
+     *            column position to move
+     * @param toColumnPosition
+     *            position to move the column to
+     */
     public void reorderColumnPosition(int fromColumnPosition, int toColumnPosition) {
         boolean reorderToLeftEdge;
         if (toColumnPosition < getColumnCount()) {
@@ -291,11 +365,33 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
         reorderColumnPosition(fromColumnPosition, toColumnPosition, reorderToLeftEdge);
     }
 
+    /**
+     * Reorders the given from-column to the specified edge of the column to
+     * move to and fires a {@link ColumnReorderEvent}.
+     *
+     * @param fromColumnPosition
+     *            column position to move
+     * @param toColumnPosition
+     *            position to move the column to
+     * @param reorderToLeftEdge
+     *            <code>true</code> if the column should be moved to the left of
+     *            the given column to move to, <code>false</code> if it should
+     *            be positioned to the right
+     */
     public void reorderColumnPosition(int fromColumnPosition, int toColumnPosition, boolean reorderToLeftEdge) {
         moveColumn(fromColumnPosition, toColumnPosition, reorderToLeftEdge);
         fireLayerEvent(new ColumnReorderEvent(this, fromColumnPosition, toColumnPosition, reorderToLeftEdge));
     }
 
+    /**
+     * Reorders the given from-columns to the <b>left</b> edge of the column to
+     * move to.
+     *
+     * @param fromColumnPositions
+     *            column positions to move
+     * @param toColumnPosition
+     *            position to move the columns to
+     */
     public void reorderMultipleColumnPositions(List<Integer> fromColumnPositions, int toColumnPosition) {
         boolean reorderToLeftEdge;
         if (toColumnPosition < getColumnCount()) {
@@ -307,6 +403,19 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
         reorderMultipleColumnPositions(fromColumnPositions, toColumnPosition, reorderToLeftEdge);
     }
 
+    /**
+     * Reorders the given from-columns to the specified edge of the column to
+     * move to and fires a {@link ColumnReorderEvent}.
+     *
+     * @param fromColumnPositions
+     *            column positions to move
+     * @param toColumnPosition
+     *            position to move the columns to
+     * @param reorderToLeftEdge
+     *            <code>true</code> if the columns should be moved to the left
+     *            of the given column to move to, <code>false</code> if they
+     *            should be positioned to the right
+     */
     public void reorderMultipleColumnPositions(List<Integer> fromColumnPositions, int toColumnPosition, boolean reorderToLeftEdge) {
         // Moving from left to right
         final int fromColumnPositionsCount = fromColumnPositions.size();
@@ -333,14 +442,32 @@ public class ColumnReorderLayer extends AbstractLayerTransform implements IUniqu
         fireLayerEvent(new ColumnReorderEvent(this, fromColumnPositions, toColumnPosition, reorderToLeftEdge));
     }
 
+    /**
+     * Clear the internal cache.
+     */
     private void invalidateCache() {
         this.startXCache.clear();
     }
 
+    /**
+     * Returns the column position from where the reorder process started. Used
+     * by the {@link ColumnReorderEndCommandHandler} which is triggered by the
+     * {@link ColumnReorderDragMode} when dragging a column is finished.
+     *
+     * @return The column position where the reorder started.
+     */
     public int getReorderFromColumnPosition() {
         return this.reorderFromColumnPosition;
     }
 
+    /**
+     * Sets the column position where a reorder process started. Typically done
+     * by calling the {@link ColumnReorderStartCommand} which is triggered by
+     * the {@link ColumnReorderDragMode}.
+     *
+     * @param fromColumnPosition
+     *            The column position where the reorder started.
+     */
     public void setReorderFromColumnPosition(int fromColumnPosition) {
         this.reorderFromColumnPosition = fromColumnPosition;
     }
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/RowReorderLayer.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/RowReorderLayer.java
index b2d74c08..69fdb38c 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/RowReorderLayer.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/reorder/RowReorderLayer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013 Dirk Fauth and others.
+ * Copyright (c) 2013, 2016 Dirk Fauth and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -41,14 +41,13 @@ import org.eclipse.nebula.widgets.nattable.reorder.config.DefaultRowReorderLayer
 import org.eclipse.nebula.widgets.nattable.reorder.event.RowReorderEvent;
 
 /**
- * Adds functionality for reordering rows(s) Also responsible for saving/loading
- * the row order state.
+ * Layer that is used to add the functionality for row reordering.
  *
  * @see DefaultRowReorderLayerConfiguration
  */
 public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIndexLayer {
 
-    private static final Log log = LogFactory.getLog(RowReorderLayer.class);
+    private static final Log LOG = LogFactory.getLog(RowReorderLayer.class);
 
     public static final String PERSISTENCE_KEY_ROW_INDEX_ORDER = ".rowIndexOrder"; //$NON-NLS-1$
 
@@ -62,6 +61,15 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
     protected final List<Integer> rowIndexOrder = new ArrayList<Integer>();
 
     /**
+     * The internal mapping of index to position values. Used for performance
+     * reasons in {@link #getColumnPositionByIndex(int)} because
+     * {@link List#indexOf(Object)} doesn't scale well.
+     *
+     * @since 1.5
+     */
+    protected final Map<Integer, Integer> indexPositionMapping = new HashMap<Integer, Integer>();
+
+    /**
      * Caching of the starting y positions of the rows. Used to reduce
      * calculation time on rendering
      */
@@ -72,10 +80,28 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
      */
     private int reorderFromRowPosition;
 
+    /**
+     * Creates a {@link RowReorderLayer} on top of the given
+     * {@link IUniqueIndexLayer} and adds the
+     * {@link DefaultRowReorderLayerConfiguration}.
+     *
+     * @param underlyingLayer
+     *            The underlying layer.
+     */
     public RowReorderLayer(IUniqueIndexLayer underlyingLayer) {
         this(underlyingLayer, true);
     }
 
+    /**
+     * Creates a {@link RowReorderLayer} on top of the given
+     * {@link IUniqueIndexLayer}.
+     *
+     * @param underlyingLayer
+     *            The underlying layer.
+     * @param useDefaultConfiguration
+     *            <code>true</code> to add the
+     *            {@link DefaultRowReorderLayerConfiguration}
+     */
     public RowReorderLayer(IUniqueIndexLayer underlyingLayer, boolean useDefaultConfiguration) {
         super(underlyingLayer);
         this.underlyingLayer = underlyingLayer;
@@ -97,12 +123,15 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
                 Collection<StructuralDiff> structuralDiffs = structuralChangeEvent.getRowDiffs();
                 if (structuralDiffs == null) {
                     // Assume everything changed
-                    this.rowIndexOrder.clear();
                     populateIndexOrder();
                 } else {
                     // only react on ADD or DELETE and not on CHANGE
-                    StructuralChangeEventHelper.handleRowDelete(structuralDiffs, this.underlyingLayer, this.rowIndexOrder, true);
-                    StructuralChangeEventHelper.handleRowInsert(structuralDiffs, this.underlyingLayer, this.rowIndexOrder, true);
+                    StructuralChangeEventHelper.handleRowDelete(
+                            structuralDiffs, this.underlyingLayer, this.rowIndexOrder, true);
+                    StructuralChangeEventHelper.handleRowInsert(
+                            structuralDiffs, this.underlyingLayer, this.rowIndexOrder, true);
+                    // update index-position mapping
+                    refreshIndexPositionMapping();
                 }
                 invalidateCache();
             }
@@ -151,6 +180,8 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
             if (isRestoredStateValid(newRowIndexOrder)) {
                 this.rowIndexOrder.clear();
                 this.rowIndexOrder.addAll(newRowIndexOrder);
+                // refresh index-position mapping
+                refreshIndexPositionMapping();
             }
 
         }
@@ -166,7 +197,7 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
      */
     protected boolean isRestoredStateValid(List<Integer> newRowIndexOrder) {
         if (newRowIndexOrder.size() != getRowCount()) {
-            log.error("Number of persisted rows (" + newRowIndexOrder.size() + ") " + //$NON-NLS-1$ //$NON-NLS-2$
+            LOG.error("Number of persisted rows (" + newRowIndexOrder.size() + ") " + //$NON-NLS-1$ //$NON-NLS-2$
                     "is not the same as the number of rows in the data source (" //$NON-NLS-1$
                     + getRowCount() + ").\n" + //$NON-NLS-1$
                     "Skipping restore of row ordering"); //$NON-NLS-1$
@@ -174,8 +205,8 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
         }
 
         for (Integer index : newRowIndexOrder) {
-            if (!this.rowIndexOrder.contains(index)) {
-                log.error("Row index: " + index + " being restored, is not a available in the data soure.\n" + //$NON-NLS-1$ //$NON-NLS-2$
+            if (!this.indexPositionMapping.containsKey(index)) {
+                LOG.error("Row index: " + index + " being restored, is not a available in the data soure.\n" + //$NON-NLS-1$ //$NON-NLS-2$
                         "Skipping restore of row ordering"); //$NON-NLS-1$
                 return false;
             }
@@ -217,9 +248,24 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
      * Initially populate the index order to the local cache.
      */
     private void populateIndexOrder() {
+        this.rowIndexOrder.clear();
         ILayer underlyingLayer = getUnderlyingLayer();
         for (int rowPosition = 0; rowPosition < underlyingLayer.getRowCount(); rowPosition++) {
-            this.rowIndexOrder.add(underlyingLayer.getRowIndexByPosition(rowPosition));
+            int index = underlyingLayer.getRowIndexByPosition(rowPosition);
+            this.rowIndexOrder.add(index);
+            this.indexPositionMapping.put(index, rowPosition);
+        }
+    }
+
+    /**
+     * Initializes the internal index-position-mapping to reflect the internal
+     * row-index-order.
+     */
+    private void refreshIndexPositionMapping() {
+        this.indexPositionMapping.clear();
+        for (int position = 0; position < this.rowIndexOrder.size(); position++) {
+            int index = this.rowIndexOrder.get(position);
+            this.indexPositionMapping.put(index, position);
         }
     }
 
@@ -244,7 +290,8 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
 
     @Override
     public int getRowPositionByIndex(int rowIndex) {
-        return this.rowIndexOrder.indexOf(Integer.valueOf(rowIndex));
+        Integer result = this.indexPositionMapping.get(rowIndex);
+        return (result != null) ? result : -1;
     }
 
     @Override
@@ -293,8 +340,11 @@ public class RowReorderLayer extends AbstractLayerTransform implements IUniqueIn
 
         Integer fromRowIndex = this.rowIndexOrder.get(fromRowPosition);
         this.rowIndexOrder.add(toRowPosition, fromRowIndex);
-
         this.rowIndexOrder.remove(fromRowPosition + (fromRowPosition > toRowPosition ? 1 : 0));
+
+        // update index-position mapping
+        refreshIndexPositionMapping();
+
         invalidateCache();
     }
 
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/SelectionModel.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/SelectionModel.java
index 86a473be..d953f452 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/SelectionModel.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/SelectionModel.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2013, 2014 Original authors and others.
+ * Copyright (c) 2012, 2016 Original authors and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -87,7 +87,6 @@ public class SelectionModel implements ISelectionModel {
         if (range != null) {
             addSelectionIntoList(range);
         }
-
     }
 
     private void addSelectionIntoList(Rectangle selection) {
@@ -281,13 +280,56 @@ public class SelectionModel implements ISelectionModel {
         return ObjectUtils.asIntArray(selectedColumns);
     }
 
+    /**
+     * @since 1.5
+     */
+    protected Set<Range> internalGetSelectedColumnPositions() {
+        Set<Range> selectedColumnsRange = new HashSet<Range>();
+
+        this.selectionsLock.readLock().lock();
+
+        int columnCount = this.selectionLayer.getColumnCount();
+        try {
+            for (Rectangle r : this.selections) {
+                if (r.x < columnCount) {
+                    int width = (r.x + r.width <= columnCount) ? r.width : columnCount - r.x;
+                    selectedColumnsRange.add(new Range(r.x, r.x + width));
+                }
+            }
+        } finally {
+            this.selectionsLock.readLock().unlock();
+        }
+
+        ArrayList<Range> ranges = new ArrayList<Range>(selectedColumnsRange);
+        Range.sortByStart(ranges);
+        List<Range> uniqueRanges = new ArrayList<Range>();
+
+        // Adjust for overlaps - between consecutive selections
+        for (int i = 0; i < ranges.size(); i++) {
+            if (i > 0) {
+                Range previousRange = ranges.get(i - 1);
+                Range currentRange = ranges.get(i);
+                if (previousRange.overlap(currentRange)
+                        || (previousRange.end == currentRange.start)) {
+                    int largerRangeEnd = (previousRange.end > currentRange.end) ? previousRange.end : currentRange.end;
+                    uniqueRanges.get(uniqueRanges.size() - 1).end = largerRangeEnd;
+                    ranges.get(i).end = largerRangeEnd;
+                } else {
+                    uniqueRanges.add(ranges.get(i));
+                }
+            } else {
+                uniqueRanges.add(ranges.get(i));
+            }
+        }
+        return new HashSet<Range>(uniqueRanges);
+    }
+
     @Override
     public boolean isColumnPositionSelected(int columnPosition) {
         this.selectionsLock.readLock().lock();
-
         try {
-            for (int column : getSelectedColumnPositions()) {
-                if (column == columnPosition) {
+            for (Range columnRange : internalGetSelectedColumnPositions()) {
+                if (columnRange.contains(columnPosition)) {
                     return true;
                 }
             }
@@ -331,7 +373,7 @@ public class SelectionModel implements ISelectionModel {
 
             // If X is same add up the height of the selected area
             for (Rectangle r : this.selections) {
-                // Column is within the bounds of the selcted rectangle
+                // Column is within the bounds of the selected rectangle
                 if (columnPosition >= r.x && columnPosition < r.x + r.width) {
                     selectedRectanglesInColumn.add(new Rectangle(
                             columnPosition,
@@ -365,7 +407,7 @@ public class SelectionModel implements ISelectionModel {
                         return false;
                     }
                 }
-                // Union will resolve any overlaping area
+                // Union will resolve any overlapping area
                 finalRectangle = finalRectangle.union(rectangle);
             }
             return finalRectangle.height >= columnHeight;
@@ -473,7 +515,7 @@ public class SelectionModel implements ISelectionModel {
 
             // If X is same add up the width of the selected area
             for (Rectangle r : this.selections) {
-                // Row is within the bounds of the selcted rectangle
+                // Row is within the bounds of the selected rectangle
                 if (rowPosition >= r.y && rowPosition < r.y + r.height) {
                     selectedRectanglesInRow.add(new Rectangle(
                             r.x,
@@ -506,7 +548,7 @@ public class SelectionModel implements ISelectionModel {
                         return false;
                     }
                 }
-                // Union will resolve any overlaping area
+                // Union will resolve any overlapping area
                 finalRectangle = finalRectangle.union(rectangle);
             }
             return finalRectangle.width >= rowWidth;
@@ -621,8 +663,7 @@ public class SelectionModel implements ISelectionModel {
                             break;
                         }
                     }
-                }
-                else {
+                } else {
                     for (StructuralDiff diff : event.getColumnDiffs()) {
                         // DiffTypeEnum.CHANGE is used for resizing and
                         // shouldn't result in clearing the selection
@@ -662,8 +703,7 @@ public class SelectionModel implements ISelectionModel {
                     }
                 }
             }
-        }
-        else {
+        } else {
             // keep the selection as is in case of changes
             // Note:
             // this is the the same code I posted in various forums as a