accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] branch master updated: Improve some comparators using Java 8 (#501)
Date Thu, 31 May 2018 19:17:19 GMT
This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
     new dc7897d  Improve some comparators using Java 8 (#501)
dc7897d is described below

commit dc7897de5e8c211a91a7a51ca0d55fbc0aa12a72
Author: Keith Turner <keith@deenlo.com>
AuthorDate: Thu May 31 15:17:00 2018 -0400

    Improve some comparators using Java 8 (#501)
---
 .../core/client/impl/ConditionalWriterImpl.java    | 37 +++----------------
 .../core/client/impl/TabletLocatorImpl.java        | 43 +++++++---------------
 .../java/org/apache/accumulo/core/data/Column.java | 26 +++++--------
 .../apache/accumulo/core/data/impl/KeyExtent.java  | 33 +++++------------
 .../accumulo/core/iterators/IteratorUtil.java      | 16 ++------
 .../core/iterators/system/HeapIterator.java        | 12 +-----
 .../client/impl/ConditionalComparatorTest.java     |  5 ++-
 .../core/client/impl/TabletLocatorImplTest.java    |  2 +-
 .../apache/accumulo/tserver/CompactionQueue.java   | 18 ++++-----
 9 files changed, 54 insertions(+), 138 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java
b/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java
index e4c60c6..fb941eb 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java
@@ -778,38 +778,13 @@ class ConditionalWriterImpl implements ConditionalWriter {
     }
   }
 
-  static class ConditionComparator implements Comparator<Condition> {
+  private static final Comparator<Long> TIMESTAMP_COMPARATOR = Comparator
+      .nullsFirst(Comparator.reverseOrder());
 
-    private static final Long MAX = Long.MAX_VALUE;
-
-    @Override
-    public int compare(Condition c1, Condition c2) {
-      int comp = c1.getFamily().compareTo(c2.getFamily());
-      if (comp == 0) {
-        comp = c1.getQualifier().compareTo(c2.getQualifier());
-        if (comp == 0) {
-          comp = c1.getVisibility().compareTo(c2.getVisibility());
-          if (comp == 0) {
-            Long l1 = c1.getTimestamp();
-            Long l2 = c2.getTimestamp();
-            if (l1 == null) {
-              l1 = MAX;
-            }
-
-            if (l2 == null) {
-              l2 = MAX;
-            }
-
-            comp = l2.compareTo(l1);
-          }
-        }
-      }
-
-      return comp;
-    }
-  }
-
-  private static final ConditionComparator CONDITION_COMPARATOR = new ConditionComparator();
+  static final Comparator<Condition> CONDITION_COMPARATOR = Comparator
+      .comparing(Condition::getFamily).thenComparing(Condition::getQualifier)
+      .thenComparing(Condition::getVisibility)
+      .thenComparing(Condition::getTimestamp, TIMESTAMP_COMPARATOR);
 
   private List<TCondition> convertConditions(ConditionalMutation cm,
       CompressedIterators compressedIters) {
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java
b/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java
index 6a296d6..746ae5d 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java
@@ -18,7 +18,6 @@ package org.apache.accumulo.core.client.impl;
 
 import static org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly;
 
-import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -57,40 +56,24 @@ public class TabletLocatorImpl extends TabletLocator {
 
   private static final Logger log = LoggerFactory.getLogger(TabletLocatorImpl.class);
 
-  // there seems to be a bug in TreeMap.tailMap related to
-  // putting null in the treemap.. therefore instead of
-  // putting null, put MAX_TEXT
+  // MAX_TEXT represents a TEXT object that is greater than all others. Attempted to use
null for
+  // this purpose, but there seems to be a bug in TreeMap.tailMap with null. Therefore instead
of
+  // using null, created MAX_TEXT.
   static final Text MAX_TEXT = new Text();
 
-  private static class EndRowComparator implements Comparator<Text>, Serializable {
-
-    private static final long serialVersionUID = 1L;
-
-    @Override
-    public int compare(Text o1, Text o2) {
-
-      int ret;
-
-      if (o1 == MAX_TEXT)
-        if (o2 == MAX_TEXT)
-          ret = 0;
-        else
-          ret = 1;
-      else if (o2 == MAX_TEXT)
-        ret = -1;
-      else
-        ret = o1.compareTo(o2);
-
-      return ret;
-    }
-
-  }
-
-  static final EndRowComparator endRowComparator = new EndRowComparator();
+  static final Comparator<Text> END_ROW_COMPARATOR = (o1, o2) -> {
+    if (o1 == o2)
+      return 0;
+    if (o1 == MAX_TEXT)
+      return 1;
+    if (o2 == MAX_TEXT)
+      return -1;
+    return o1.compareTo(o2);
+  };
 
   protected Table.ID tableId;
   protected TabletLocator parent;
-  protected TreeMap<Text,TabletLocation> metaCache = new TreeMap<>(endRowComparator);
+  protected TreeMap<Text,TabletLocation> metaCache = new TreeMap<>(END_ROW_COMPARATOR);
   protected TabletLocationObtainer locationObtainer;
   private TabletServerLockChecker lockChecker;
   protected Text lastTabletRow;
diff --git a/core/src/main/java/org/apache/accumulo/core/data/Column.java b/core/src/main/java/org/apache/accumulo/core/data/Column.java
index a95f2cd..6752f60 100644
--- a/core/src/main/java/org/apache/accumulo/core/data/Column.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/Column.java
@@ -23,6 +23,7 @@ import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.util.Comparator;
 
 import org.apache.accumulo.core.data.thrift.TColumn;
 import org.apache.hadoop.io.WritableComparable;
@@ -33,15 +34,13 @@ import org.apache.hadoop.io.WritableComparator;
  */
 public class Column implements WritableComparable<Column> {
 
-  static private int compareBytes(byte[] a, byte[] b) {
-    if (a == null && b == null)
-      return 0;
-    if (a == null)
-      return -1;
-    if (b == null)
-      return 1;
-    return WritableComparator.compareBytes(a, 0, a.length, b, 0, b.length);
-  }
+  private static final Comparator<byte[]> BYTE_COMPARATOR = Comparator
+      .nullsFirst((a, b) -> WritableComparator.compareBytes(a, 0, a.length, b, 0, b.length));
+
+  private static final Comparator<Column> COMPARATOR = Comparator
+      .comparing(Column::getColumnFamily, BYTE_COMPARATOR)
+      .thenComparing(Column::getColumnQualifier, BYTE_COMPARATOR)
+      .thenComparing(Column::getColumnVisibility, BYTE_COMPARATOR);
 
   /**
    * Compares this column to another. Column families are compared first, then qualifiers,
then
@@ -53,14 +52,7 @@ public class Column implements WritableComparable<Column> {
    */
   @Override
   public int compareTo(Column that) {
-    int result;
-    result = compareBytes(this.columnFamily, that.columnFamily);
-    if (result != 0)
-      return result;
-    result = compareBytes(this.columnQualifier, that.columnQualifier);
-    if (result != 0)
-      return result;
-    return compareBytes(this.columnVisibility, that.columnVisibility);
+    return COMPARATOR.compare(this, that);
   }
 
   @Override
diff --git a/core/src/main/java/org/apache/accumulo/core/data/impl/KeyExtent.java b/core/src/main/java/org/apache/accumulo/core/data/impl/KeyExtent.java
index d4833d5..c20ad52 100644
--- a/core/src/main/java/org/apache/accumulo/core/data/impl/KeyExtent.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/impl/KeyExtent.java
@@ -28,6 +28,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Set;
@@ -383,32 +384,16 @@ public class KeyExtent implements WritableComparable<KeyExtent>
{
     return m;
   }
 
+  // The last tablet in a table has no end row, so null sorts last for end row; similarly,
the first
+  // tablet has no previous end row, so null sorts first for previous end row
+  private static final Comparator<KeyExtent> COMPARATOR = Comparator
+      .comparing(KeyExtent::getTableId)
+      .thenComparing(KeyExtent::getEndRow, Comparator.nullsLast(Text::compareTo))
+      .thenComparing(KeyExtent::getPrevEndRow, Comparator.nullsFirst(Text::compareTo));
+
   @Override
   public int compareTo(KeyExtent other) {
-
-    int result = getTableId().compareTo(other.getTableId());
-    if (result != 0)
-      return result;
-
-    if (this.getEndRow() == null) {
-      if (other.getEndRow() != null)
-        return 1;
-    } else {
-      if (other.getEndRow() == null)
-        return -1;
-
-      result = getEndRow().compareTo(other.getEndRow());
-      if (result != 0)
-        return result;
-    }
-    if (this.getPrevEndRow() == null) {
-      if (other.getPrevEndRow() == null)
-        return 0;
-      return -1;
-    }
-    if (other.getPrevEndRow() == null)
-      return 1;
-    return this.getPrevEndRow().compareTo(other.getPrevEndRow());
+    return COMPARATOR.compare(this, other);
   }
 
   private int hashCode = 0;
diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java b/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java
index 1a37b7c..d6e904c 100644
--- a/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java
@@ -19,7 +19,6 @@ package org.apache.accumulo.core.iterators;
 import static java.util.Objects.requireNonNull;
 
 import java.io.IOException;
-import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -72,15 +71,8 @@ public class IteratorUtil {
     majc, minc, scan;
   }
 
-  public static class IterInfoComparator implements Comparator<IterInfo>, Serializable
{
-    private static final long serialVersionUID = 1L;
-
-    @Override
-    public int compare(IterInfo o1, IterInfo o2) {
-      return (o1.priority < o2.priority ? -1 : (o1.priority == o2.priority ? 0 : 1));
-    }
-
-  }
+  private static Comparator<IterInfo> ITER_INFO_COMPARATOR = Comparator
+      .comparingInt(IterInfo::getPriority);
 
   /**
    * Fetch the correct configuration key prefix for the given scope. Throws an
@@ -130,7 +122,7 @@ public class IteratorUtil {
       Map<String,Map<String,String>> ssio) {
     destList.addAll(tableIters);
     destList.addAll(ssi);
-    Collections.sort(destList, new IterInfoComparator());
+    Collections.sort(destList, ITER_INFO_COMPARATOR);
 
     Set<Entry<String,Map<String,String>>> es = tableOpts.entrySet();
     for (Entry<String,Map<String,String>> entry : es) {
@@ -176,7 +168,7 @@ public class IteratorUtil {
       }
     }
 
-    Collections.sort(iters, new IterInfoComparator());
+    Collections.sort(iters, ITER_INFO_COMPARATOR);
   }
 
   // @formatter:off
diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/system/HeapIterator.java
b/core/src/main/java/org/apache/accumulo/core/iterators/system/HeapIterator.java
index 815b247..43c15d9 100644
--- a/core/src/main/java/org/apache/accumulo/core/iterators/system/HeapIterator.java
+++ b/core/src/main/java/org/apache/accumulo/core/iterators/system/HeapIterator.java
@@ -17,7 +17,6 @@
 package org.apache.accumulo.core.iterators.system;
 
 import java.io.IOException;
-import java.util.Comparator;
 import java.util.PriorityQueue;
 
 import org.apache.accumulo.core.data.Key;
@@ -33,14 +32,6 @@ public abstract class HeapIterator implements SortedKeyValueIterator<Key,Value>
   private SortedKeyValueIterator<Key,Value> topIdx = null;
   private Key nextKey;
 
-  private static class SKVIComparator implements Comparator<SortedKeyValueIterator<Key,Value>>
{
-
-    @Override
-    public int compare(SortedKeyValueIterator<Key,Value> o1, SortedKeyValueIterator<Key,Value>
o2) {
-      return o1.getTopKey().compareTo(o2.getTopKey());
-    }
-  }
-
   protected HeapIterator() {
     heap = null;
   }
@@ -53,7 +44,8 @@ public abstract class HeapIterator implements SortedKeyValueIterator<Key,Value>
     if (heap != null)
       throw new IllegalStateException("heap already exist");
 
-    heap = new PriorityQueue<>(maxSize == 0 ? 1 : maxSize, new SKVIComparator());
+    heap = new PriorityQueue<>(maxSize == 0 ? 1 : maxSize,
+        (si1, si2) -> si1.getTopKey().compareTo(si2.getTopKey()));
   }
 
   @Override
diff --git a/core/src/test/java/org/apache/accumulo/core/client/impl/ConditionalComparatorTest.java
b/core/src/test/java/org/apache/accumulo/core/client/impl/ConditionalComparatorTest.java
index f44f929..b8ba83c 100644
--- a/core/src/test/java/org/apache/accumulo/core/client/impl/ConditionalComparatorTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/client/impl/ConditionalComparatorTest.java
@@ -17,7 +17,8 @@
 
 package org.apache.accumulo.core.client.impl;
 
-import org.apache.accumulo.core.client.impl.ConditionalWriterImpl.ConditionComparator;
+import java.util.Comparator;
+
 import org.apache.accumulo.core.data.Condition;
 import org.apache.accumulo.core.security.ColumnVisibility;
 import org.junit.Assert;
@@ -34,7 +35,7 @@ public class ConditionalComparatorTest {
     Condition c6 = new Condition("a", "b").setVisibility(new ColumnVisibility("A&B"));
     Condition c7 = new Condition("a", "b").setVisibility(new ColumnVisibility("A&C"));
 
-    ConditionComparator comparator = new ConditionComparator();
+    Comparator<Condition> comparator = ConditionalWriterImpl.CONDITION_COMPARATOR;
 
     Assert.assertEquals(0, comparator.compare(c1, c1));
     Assert.assertTrue(comparator.compare(c1, c2) < 0);
diff --git a/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java
b/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java
index fd571b0..30908f9 100644
--- a/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java
@@ -124,7 +124,7 @@ public class TabletLocatorImplTest {
   static TreeMap<Text,TabletLocation> createMetaCache(Object... data) {
     TreeMap<KeyExtent,TabletLocation> mcke = createMetaCacheKE(data);
 
-    TreeMap<Text,TabletLocation> mc = new TreeMap<>(TabletLocatorImpl.endRowComparator);
+    TreeMap<Text,TabletLocation> mc = new TreeMap<>(TabletLocatorImpl.END_ROW_COMPARATOR);
 
     for (Entry<KeyExtent,TabletLocation> entry : mcke.entrySet()) {
       if (entry.getKey().getEndRow() == null)
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionQueue.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionQueue.java
index 2b40616..1d90e70 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionQueue.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionQueue.java
@@ -36,20 +36,16 @@ class CompactionQueue extends AbstractQueue<TraceRunnable> implements
BlockingQu
 
   private List<TraceRunnable> task = new LinkedList<>();
 
-  private static final Comparator<TraceRunnable> comparator = new Comparator<TraceRunnable>()
{
-    @SuppressWarnings("unchecked")
-    @Override
-    public int compare(TraceRunnable o1, TraceRunnable o2) {
-      return ((Comparable<Runnable>) o1.getRunnable()).compareTo(o2.getRunnable());
-    }
-  };
+  @SuppressWarnings("unchecked")
+  private static final Comparator<TraceRunnable> ELEMENT_COMPARATOR = (o1,
+      o2) -> ((Comparable<Runnable>) o1.getRunnable()).compareTo(o2.getRunnable());
 
   @Override
   public synchronized TraceRunnable poll() {
     if (task.size() == 0)
       return null;
 
-    TraceRunnable min = Collections.min(task, comparator);
+    TraceRunnable min = Collections.min(task, ELEMENT_COMPARATOR);
     Iterator<TraceRunnable> iterator = task.iterator();
     while (iterator.hasNext()) {
       if (iterator.next() == min) {
@@ -65,7 +61,7 @@ class CompactionQueue extends AbstractQueue<TraceRunnable> implements
BlockingQu
     if (task.size() == 0)
       return null;
 
-    return Collections.min(task, comparator);
+    return Collections.min(task, ELEMENT_COMPARATOR);
   }
 
   @Override
@@ -122,7 +118,7 @@ class CompactionQueue extends AbstractQueue<TraceRunnable> implements
BlockingQu
 
   @Override
   public synchronized int drainTo(Collection<? super TraceRunnable> c, int maxElements)
{
-    Collections.sort(task, comparator);
+    Collections.sort(task, ELEMENT_COMPARATOR);
 
     int num = Math.min(task.size(), maxElements);
 
@@ -137,7 +133,7 @@ class CompactionQueue extends AbstractQueue<TraceRunnable> implements
BlockingQu
 
   @Override
   public synchronized Iterator<TraceRunnable> iterator() {
-    Collections.sort(task, comparator);
+    Collections.sort(task, ELEMENT_COMPARATOR);
 
     return task.iterator();
   }

-- 
To stop receiving notification emails like this one, please contact
kturner@apache.org.

Mime
View raw message