[SPARK-7542] [SQL] Support off-heap index/sort buffer #9477

davies · 2015-11-05T00:08:06Z

This brings the support of off-heap memory for array inside BytesToBytesMap and InMemorySorter, then we could allocate all the memory from off-heap for execution.

Closes #8068

SparkQA · 2015-11-05T03:05:04Z

Test build #45072 has finished for PR 9477 at commit 77555e1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-11-05T04:08:33Z

Test build #45076 has finished for PR 9477 at commit 3cb22d4.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

SparkQA · 2015-11-05T04:30:27Z

Test build #45070 has finished for PR 9477 at commit 862b38f.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds no public classes.

davies · 2015-11-05T05:34:06Z

@JoshRosen This is ready for review

JoshRosen · 2015-11-05T07:22:47Z

core/src/main/java/org/apache/spark/shuffle/sort/ShuffleExternalSorter.java

      try {
-        acquireMemory(needed);  // could trigger spilling
+        // could trigger spilling
+        page = taskMemoryManager.allocatePage(used * 2, this);


Implicit here is the fact that the in memory sorter's only source of memory usage is the pointer array itself. That's fine, though.

JoshRosen · 2015-11-05T07:43:10Z

LGTM overall, but I'd like to address one concern before merging: I'm worried that passing both the MemoryConsumer and TaskMemoryManager to the sorter components will open the potential for bugs; I feel that those classes should allocate their memory using only the public MemoryConsumer methods.

Note that the MemoryConsumer methods actually end up calling the TaskMemoryManager methods which your code calls directly:

spark/core/src/main/java/org/apache/spark/memory/MemoryConsumer.java

Line 81 in 81498dd

protected void acquireMemory(long size) {

I think that we should mark those TaskMemoryManager methods as methods that are only supposed to be called by the memory consumer and not directly by developers. The problem with calling them directly is that the bookkeeping in the MemoryConsumer itself won't have been updated. This current API has such a high potential for this type of misuse that I think we should look at fancy Java-isms to restrict the visibility / callability of those methods such that they can only be called from MemoryConsumer.

If you fix this by having those classes only allocate through their MemoryConsumers then feel free to merge as soon as this passes tests. I can take care of a followup to fix the documentation / code to avoid this type of misuse.

SparkQA · 2015-11-05T08:08:07Z

Test build #1983 has finished for PR 9477 at commit 3cb22d4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

davies · 2015-11-05T08:44:41Z

@JoshRosen I should had addressed your comments, will merge this once pass the tests.

SparkQA · 2015-11-05T09:04:48Z

Test build #45096 has finished for PR 9477 at commit 89319e0.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

SparkQA · 2015-11-05T12:44:08Z

Test build #45108 has finished for PR 9477 at commit b42e7db.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

SparkQA · 2015-11-05T13:05:59Z

Test build #45110 has finished for PR 9477 at commit 854a99f.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

JoshRosen · 2015-11-05T17:45:25Z

Jenkins, retest this please.

JoshRosen · 2015-11-05T17:49:39Z

core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java

      try {
-        acquireMemory(needed);  // could trigger spilling
+        // could trigger spilling
+        array = allocateArray(used / 16 * 2);


Should this be / 8 * 2 instead, since we want to double the number of slots in the array and each slot requires 8 bytes of memory?

SparkQA · 2015-11-05T22:06:00Z

Test build #45125 has finished for PR 9477 at commit 854a99f.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

SparkQA · 2015-11-05T22:16:32Z

Test build #45127 has finished for PR 9477 at commit c35f512.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

SparkQA · 2015-11-05T23:04:14Z

Test build #1989 has finished for PR 9477 at commit b367daf.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

davies · 2015-11-05T23:23:42Z

The failed test is not related, will re-run it.

SparkQA · 2015-11-06T02:03:45Z

Test build #1993 has finished for PR 9477 at commit b367daf.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

JoshRosen · 2015-11-06T03:01:20Z

LGTM, so I'm going to merge this into master and 1.6. Thanks!

This brings the support of off-heap memory for array inside BytesToBytesMap and InMemorySorter, then we could allocate all the memory from off-heap for execution. Closes #8068 Author: Davies Liu <davies@databricks.com> Closes #9477 from davies/unsafe_timsort. (cherry picked from commit eec74ba) Signed-off-by: Josh Rosen <joshrosen@databricks.com>

SparkQA · 2015-11-06T03:26:15Z

Test build #1994 has finished for PR 9477 at commit b367daf.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class ShuffleSortDataFormat extends SortDataFormat<PackedRecordPointer, LongArray>\n * final class UnsafeSortDataFormat extends SortDataFormat<RecordPointerAndKeyPrefix, LongArray>\n

davies force-pushed the unsafe_timsort branch 3 times, most recently from 77555e1 to 3cb22d4 Compare November 5, 2015 01:24

support off-heap index/sort buffer

89319e0

davies force-pushed the unsafe_timsort branch from 3cb22d4 to 89319e0 Compare November 5, 2015 05:33

JoshRosen reviewed Nov 5, 2015
View reviewed changes

Davies Liu added 2 commits November 5, 2015 00:31

address comments

b42e7db

refactor

854a99f

JoshRosen reviewed Nov 5, 2015
View reviewed changes

Davies Liu added 2 commits November 5, 2015 10:01

bugfix

c35f512

fix test

b367daf

asfgit closed this in eec74ba Nov 6, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-7542] [SQL] Support off-heap index/sort buffer #9477

[SPARK-7542] [SQL] Support off-heap index/sort buffer #9477

davies commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

davies commented Nov 5, 2015

JoshRosen Nov 5, 2015

JoshRosen commented Nov 5, 2015

SparkQA commented Nov 5, 2015

davies commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

JoshRosen commented Nov 5, 2015

JoshRosen Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

davies commented Nov 5, 2015

SparkQA commented Nov 6, 2015

JoshRosen commented Nov 6, 2015

SparkQA commented Nov 6, 2015

[SPARK-7542] [SQL] Support off-heap index/sort buffer #9477

[SPARK-7542] [SQL] Support off-heap index/sort buffer #9477

Conversation

davies commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

davies commented Nov 5, 2015

JoshRosen Nov 5, 2015

Choose a reason for hiding this comment

JoshRosen commented Nov 5, 2015

SparkQA commented Nov 5, 2015

davies commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

JoshRosen commented Nov 5, 2015

JoshRosen Nov 5, 2015

Choose a reason for hiding this comment

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

SparkQA commented Nov 5, 2015

davies commented Nov 5, 2015

SparkQA commented Nov 6, 2015

JoshRosen commented Nov 6, 2015

SparkQA commented Nov 6, 2015