From db149d968790fb42f5a87e45245843c3005b79ae Mon Sep 17 00:00:00 2001 From: Dionysios Logothetis Date: Wed, 24 Oct 2018 14:05:55 -0700 Subject: [PATCH 1/4] Remove atomic counter overhead in LongByteMappingStore --- .../apache/giraph/mapping/LongByteMappingStore.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java b/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java index ab35cdc6f..ee4790ed5 100644 --- a/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java +++ b/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java @@ -22,7 +22,6 @@ import java.util.Arrays; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicLong; import javax.annotation.concurrent.ThreadSafe; @@ -31,7 +30,6 @@ import org.apache.hadoop.io.ByteWritable; import org.apache.hadoop.io.LongWritable; import org.apache.hadoop.io.Writable; -import org.apache.log4j.Logger; import com.google.common.collect.MapMaker; @@ -48,12 +46,6 @@ public class LongByteMappingStore extends DefaultImmutableClassesGiraphConfigurable implements MappingStore { - /** Logger instance */ - private static final Logger LOG = Logger.getLogger( - LongByteMappingStore.class); - - /** Counts number of entries added */ - private final AtomicLong numEntries = new AtomicLong(0); /** Id prefix to bytesArray index mapping */ private ConcurrentMap concurrentIdToBytes; @@ -114,7 +106,6 @@ public void addEntry(LongWritable vertexId, ByteWritable target) { } } bytes[(int) (vertexId.get() & lowerBitMask)] = target.get(); - numEntries.getAndIncrement(); // increment count } @Override @@ -140,6 +131,6 @@ public void postFilling() { @Override public long getStats() { - return numEntries.longValue(); + return concurrentIdToBytes.size(); } } From 497278cdc8aa24a18ccdc3bb701939cbe8de6fb7 Mon Sep 17 00:00:00 2001 From: Dionysios Logothetis Date: Wed, 24 Oct 2018 14:55:41 -0700 Subject: [PATCH 2/4] Get size from the right map --- .../org/apache/giraph/mapping/LongByteMappingStore.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java b/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java index ee4790ed5..68eb83b8b 100644 --- a/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java +++ b/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java @@ -129,8 +129,15 @@ public void postFilling() { concurrentIdToBytes = null; } + /** + * Returns the number of entries in the mapping store. This is updated only + * after the mapping has finished loading after {@link #postFilling()} has + * been called. + * + * @return + */ @Override public long getStats() { - return concurrentIdToBytes.size(); + return idToBytes.size(); } } From d6c60be07f0eb5296ad740bf6c6ff295aec30ccf Mon Sep 17 00:00:00 2001 From: Dionysios Logothetis Date: Thu, 25 Oct 2018 14:04:09 -0700 Subject: [PATCH 3/4] Improve getByTarget method and unit test --- .../giraph/mapping/LongByteMappingStore.java | 8 ++-- .../mapping/LongByteMappingStoreTest.java | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java diff --git a/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java b/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java index 68eb83b8b..fb429e447 100644 --- a/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java +++ b/giraph-core/src/main/java/org/apache/giraph/mapping/LongByteMappingStore.java @@ -76,6 +76,7 @@ public void initialize() { .concurrencyLevel(getConf().getNumInputSplitsThreads()) .makeMap(); idToBytes = new Long2ObjectOpenHashMap<>(upper); + idToBytes.defaultReturnValue(null); } /** @@ -86,11 +87,12 @@ public void initialize() { */ public byte getByteTarget(LongWritable vertexId) { long key = vertexId.get() >>> lowerOrder; - int suffix = (int) (vertexId.get() & lowerBitMask); - if (!idToBytes.containsKey(key)) { + byte[] bs = idToBytes.get(key); + if (bs == null) { return -1; } - return idToBytes.get(key)[suffix]; + int suffix = (int) (vertexId.get() & lowerBitMask); + return bs[suffix]; } @Override diff --git a/giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java b/giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java new file mode 100644 index 000000000..b0ccb2a67 --- /dev/null +++ b/giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java @@ -0,0 +1,38 @@ +package org.apache.giraph.mapping; + +import org.apache.giraph.conf.GiraphConfiguration; +import org.apache.giraph.conf.GiraphConstants; +import org.apache.giraph.conf.ImmutableClassesGiraphConfiguration; +import org.apache.hadoop.io.ByteWritable; +import org.apache.hadoop.io.LongWritable; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class LongByteMappingStoreTest { + + @Test + public void test() { + ImmutableClassesGiraphConfiguration conf = + new ImmutableClassesGiraphConfiguration(new GiraphConfiguration()); + GiraphConstants.LB_MAPPINGSTORE_UPPER.setIfUnset(conf, 100); + GiraphConstants.LB_MAPPINGSTORE_LOWER.setIfUnset(conf, 4); + LongByteMappingStore store = new LongByteMappingStore(); + store.setConf(conf); + store.initialize(); + store.addEntry(new LongWritable(1), new ByteWritable((byte) 1)); + store.addEntry(new LongWritable(2), new ByteWritable((byte) 2)); + store.postFilling(); + + assertEquals((byte) 1, store.getByteTarget(new LongWritable(1))); + assertEquals((byte) 2, store.getByteTarget(new LongWritable(2))); + assertEquals((byte) -1, store.getByteTarget(new LongWritable(999))); + + assertEquals(new ByteWritable((byte) 1), + store.getTarget(new LongWritable(1), new ByteWritable((byte) 777))); + assertEquals(new ByteWritable((byte) 2), + store.getTarget(new LongWritable(2), new ByteWritable((byte) 888))); + assertNull(store.getTarget(new LongWritable(3), new ByteWritable((byte) 555))); + } +} From 1dbbeda5dc010333dea40d8fa47e6fa9b14ec738 Mon Sep 17 00:00:00 2001 From: Dionysios Logothetis Date: Thu, 25 Oct 2018 14:07:13 -0700 Subject: [PATCH 4/4] Checkstyle --- .../org/apache/giraph/mapping/LongByteMappingStoreTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java b/giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java index b0ccb2a67..a23da03ff 100644 --- a/giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java +++ b/giraph-core/src/test/java/org/apache/giraph/mapping/LongByteMappingStoreTest.java @@ -33,6 +33,7 @@ public void test() { store.getTarget(new LongWritable(1), new ByteWritable((byte) 777))); assertEquals(new ByteWritable((byte) 2), store.getTarget(new LongWritable(2), new ByteWritable((byte) 888))); - assertNull(store.getTarget(new LongWritable(3), new ByteWritable((byte) 555))); + assertNull(store.getTarget(new LongWritable(3), + new ByteWritable((byte) 555))); } }