You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by kt...@apache.org on 2018/05/31 19:17:19 UTC
[accumulo] branch master updated: Improve some comparators using
Java 8 (#501)
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 <ke...@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.