From d157ea1ae8c84335f9094d1e16253ba959e3cd77 Mon Sep 17 00:00:00 2001 From: Dongjoon Hyun Date: Wed, 2 Nov 2016 15:42:41 -0700 Subject: [PATCH 1/4] [SPARK-18200][GRAPHX] Support zero-size initial capacity in OpenHashSet --- .../scala/org/apache/spark/util/collection/OpenHashSet.scala | 4 +++- .../org/apache/spark/util/collection/OpenHashSetSuite.scala | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala b/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala index 0f6a425e3db9a..0629ba658c910 100644 --- a/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala +++ b/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala @@ -42,10 +42,12 @@ import org.apache.spark.annotation.Private */ @Private class OpenHashSet[@specialized(Long, Int) T: ClassTag]( - initialCapacity: Int, + var initialCapacity: Int, loadFactor: Double) extends Serializable { + if (initialCapacity == 0) initialCapacity = 1 + require(initialCapacity <= OpenHashSet.MAX_CAPACITY, s"Can't make capacity bigger than ${OpenHashSet.MAX_CAPACITY} elements") require(initialCapacity >= 1, "Invalid initial capacity") diff --git a/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala b/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala index 2607a543dd614..4d2a2267adc38 100644 --- a/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala +++ b/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala @@ -176,4 +176,9 @@ class OpenHashSetSuite extends SparkFunSuite with Matchers { assert(set.size === 1000) assert(set.capacity > 1000) } + + test("SPARK-18200 Support zero-size initial set size") { + val set = new OpenHashSet[Long](0) + assert(set.size === 0) + } } From 28879cb4f9cd5aedf4925bff7cb530b3869dc170 Mon Sep 17 00:00:00 2001 From: Dongjoon Hyun Date: Wed, 2 Nov 2016 16:13:27 -0700 Subject: [PATCH 2/4] Address comments. --- .../org/apache/spark/util/collection/OpenHashSet.scala | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala b/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala index 0629ba658c910..3e3751be93d3a 100644 --- a/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala +++ b/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala @@ -42,15 +42,13 @@ import org.apache.spark.annotation.Private */ @Private class OpenHashSet[@specialized(Long, Int) T: ClassTag]( - var initialCapacity: Int, + initialCapacity: Int, loadFactor: Double) extends Serializable { - if (initialCapacity == 0) initialCapacity = 1 - require(initialCapacity <= OpenHashSet.MAX_CAPACITY, s"Can't make capacity bigger than ${OpenHashSet.MAX_CAPACITY} elements") - require(initialCapacity >= 1, "Invalid initial capacity") + require(initialCapacity >= 0, "Invalid initial capacity") require(loadFactor < 1.0, "Load factor must be less than 1.0") require(loadFactor > 0.0, "Load factor must be greater than 0.0") @@ -273,7 +271,7 @@ class OpenHashSet[@specialized(Long, Int) T: ClassTag]( private def hashcode(h: Int): Int = Hashing.murmur3_32().hashInt(h).asInt() private def nextPowerOf2(n: Int): Int = { - val highBit = Integer.highestOneBit(n) + val highBit = Integer.highestOneBit(if (n == 0) 1 else n) if (highBit == n) n else highBit << 1 } } From d7bd9a9923ad1042453fdad112b65db65ba8f762 Mon Sep 17 00:00:00 2001 From: Dongjoon Hyun Date: Wed, 2 Nov 2016 16:15:11 -0700 Subject: [PATCH 3/4] Fix testcase name. --- .../org/apache/spark/util/collection/OpenHashSetSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala b/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala index 4d2a2267adc38..210bc5c099742 100644 --- a/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala +++ b/core/src/test/scala/org/apache/spark/util/collection/OpenHashSetSuite.scala @@ -177,7 +177,7 @@ class OpenHashSetSuite extends SparkFunSuite with Matchers { assert(set.capacity > 1000) } - test("SPARK-18200 Support zero-size initial set size") { + test("SPARK-18200 Support zero as an initial set size") { val set = new OpenHashSet[Long](0) assert(set.size === 0) } From b4dd73ddf1dca402931775cada34641698a32901 Mon Sep 17 00:00:00 2001 From: Dongjoon Hyun Date: Wed, 2 Nov 2016 18:51:25 -0700 Subject: [PATCH 4/4] Remove testcase to expect the old behaviors. --- .../org/apache/spark/util/collection/OpenHashSet.scala | 8 ++++++-- .../apache/spark/util/collection/OpenHashMapSuite.scala | 3 --- .../util/collection/PrimitiveKeyOpenHashMapSuite.scala | 3 --- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala b/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala index 3e3751be93d3a..7a1be8515d965 100644 --- a/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala +++ b/core/src/main/scala/org/apache/spark/util/collection/OpenHashSet.scala @@ -271,8 +271,12 @@ class OpenHashSet[@specialized(Long, Int) T: ClassTag]( private def hashcode(h: Int): Int = Hashing.murmur3_32().hashInt(h).asInt() private def nextPowerOf2(n: Int): Int = { - val highBit = Integer.highestOneBit(if (n == 0) 1 else n) - if (highBit == n) n else highBit << 1 + if (n == 0) { + 2 + } else { + val highBit = Integer.highestOneBit(n) + if (highBit == n) n else highBit << 1 + } } } diff --git a/core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala b/core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala index 3066e9996abda..335ecb9320ab9 100644 --- a/core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala +++ b/core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala @@ -49,9 +49,6 @@ class OpenHashMapSuite extends SparkFunSuite with Matchers { intercept[IllegalArgumentException] { new OpenHashMap[String, Int](-1) } - intercept[IllegalArgumentException] { - new OpenHashMap[String, String](0) - } } test("primitive value") { diff --git a/core/src/test/scala/org/apache/spark/util/collection/PrimitiveKeyOpenHashMapSuite.scala b/core/src/test/scala/org/apache/spark/util/collection/PrimitiveKeyOpenHashMapSuite.scala index 508e737b725bc..f5ee428020fd4 100644 --- a/core/src/test/scala/org/apache/spark/util/collection/PrimitiveKeyOpenHashMapSuite.scala +++ b/core/src/test/scala/org/apache/spark/util/collection/PrimitiveKeyOpenHashMapSuite.scala @@ -49,9 +49,6 @@ class PrimitiveKeyOpenHashMapSuite extends SparkFunSuite with Matchers { intercept[IllegalArgumentException] { new PrimitiveKeyOpenHashMap[Int, Int](-1) } - intercept[IllegalArgumentException] { - new PrimitiveKeyOpenHashMap[Int, Int](0) - } } test("basic operations") {