Github user tedyu commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60685910
--- Diff:
core/src/test/scala/org/apache/spark/util/collection/unsafe/sort/RadixSortSuite.scala
---
@@ -0,0 +1,264 @@
+/*
+ * Licensed to the Apache
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12490
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-213162459
Merging this into master, thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-213134892
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-213134888
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-213133847
**[Test build #56581 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56581/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-213098364
**[Test build #56581 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56581/consoleFull)**
for PR 12490 at commit
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60648020
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,32 @@ object SortPrefixUtils {
}
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60647905
--- Diff:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/RadixSort.java
---
@@ -0,0 +1,253 @@
+/*
+ * Licensed to the Apache
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60647739
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Sort.scala
---
@@ -139,11 +148,15 @@ case class Sort(
val dataSize = metricTerm(ctx,
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60647734
--- Diff:
core/src/test/scala/org/apache/spark/util/collection/unsafe/sort/PrefixComparatorsSuite.scala
---
@@ -110,4 +112,12 @@ class PrefixComparatorsSuite
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212770972
LGTM overall, just some minor comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60532479
--- Diff:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/RadixSort.java
---
@@ -0,0 +1,253 @@
+/*
+ * Licensed to the Apache
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60529161
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,32 @@ object SortPrefixUtils {
}
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60529190
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Sort.scala
---
@@ -139,11 +148,15 @@ case class Sort(
val dataSize =
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60528873
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,32 @@ object SortPrefixUtils {
}
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60528592
--- Diff:
core/src/test/scala/org/apache/spark/util/collection/unsafe/sort/PrefixComparatorsSuite.scala
---
@@ -110,4 +112,12 @@ class
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60526773
--- Diff:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/PrefixComparators.java
---
@@ -28,88 +28,84 @@
public class PrefixComparators
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60526550
--- Diff:
core/src/main/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorter.java ---
@@ -87,18 +102,18 @@ public void expandPointerArray(LongArray
Github user ericl commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212691949
ptal
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212675549
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212675551
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212675352
**[Test build #56429 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56429/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212644491
**[Test build #56429 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56429/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212596602
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212596600
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212595870
**[Test build #56389 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56389/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212548929
**[Test build #56389 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56389/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212316191
**[Test build #56338 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56338/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212316213
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212316217
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212312260
**[Test build #56338 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56338/consoleFull)**
for PR 12490 at commit
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212245315
Since we are using more memory, you should update the test
UnsafeShuffleWriterSuite
---
If your project is set up for it, you can reply to this email and have your
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212245007
@ericl The precision is greater than 18, have you forgot to check the
precision somewhere?
---
If your project is set up for it, you can reply to this email and have
Github user ericl commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212209007
It seems this is the failing test. @davies any idea why the decimal
handling is wrong here? If I am understanding correctly when `dt.precision -
dt.scale <=
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212203922
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212203921
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212203825
**[Test build #56296 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56296/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212178743
**[Test build #56296 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56296/consoleFull)**
for PR 12490 at commit
Github user ericl commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212177739
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212169575
That makes sense. Can you put it in pr description as a todo for future pr?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user ericl commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212169127
I was going to punt null handling for longs into another PR, unless you
think it should be in this one? The only case that doesn't work is for LongType.
---
If your
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212166485
Eric can you put the way you handle null in the pr description, and as some
classdoc comment somewhere?
---
If your project is set up for it, you can reply to this
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212163368
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212163374
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212163351
**[Test build #56288 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56288/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212162690
**[Test build #56288 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56288/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212161545
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212161537
**[Test build #56284 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56284/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212161543
Build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327136
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,29 @@ object SortPrefixUtils {
}
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327140
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,29 @@ object SortPrefixUtils {
}
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212161313
**[Test build #56284 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56284/consoleFull)**
for PR 12490 at commit
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327131
--- Diff:
sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
---
@@ -85,13 +85,15 @@ public UnsafeKVExternalSorter(
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327152
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -88,6 +88,12 @@ object SQLConf {
.booleanConf
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327119
--- Diff:
core/src/test/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorterSuite.java
---
@@ -70,14 +73,15 @@ public void testBasicSorting() throws
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327142
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,29 @@ object SortPrefixUtils {
}
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327092
--- Diff:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/RadixSort.java
---
@@ -0,0 +1,253 @@
+/*
+ * Licensed to the Apache
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327080
--- Diff:
core/src/main/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorter.java ---
@@ -47,18 +48,25 @@ public int compare(PackedRecordPointer left,
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60327083
--- Diff:
core/src/main/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorter.java ---
@@ -148,7 +157,12 @@ public void loadNext() {
* Return an
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-212036416
@ericl can you decouple the benchmark change from this pr?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60275221
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,29 @@ object SortPrefixUtils {
}
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60268804
--- Diff:
sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
---
@@ -85,13 +85,15 @@ public UnsafeKVExternalSorter(
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60268406
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,29 @@ object SortPrefixUtils {
}
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60268294
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/SortPrefixUtils.scala ---
@@ -66,6 +66,29 @@ object SortPrefixUtils {
}
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60267980
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -88,6 +88,12 @@ object SQLConf {
.booleanConf
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60265931
--- Diff:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/RadixSort.java
---
@@ -0,0 +1,253 @@
+/*
+ * Licensed to the Apache
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60265463
--- Diff:
core/src/main/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorter.java ---
@@ -148,7 +157,12 @@ public void loadNext() {
* Return
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60264951
--- Diff:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/PrefixComparators.java
---
@@ -28,88 +28,84 @@
public class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60264341
--- Diff:
core/src/main/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorter.java ---
@@ -47,18 +48,25 @@ public int compare(PackedRecordPointer
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211785722
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211785718
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211785553
**[Test build #56201 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56201/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211784566
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211784569
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211784426
**[Test build #56200 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56200/consoleFull)**
for PR 12490 at commit
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12490#discussion_r60183184
--- Diff:
core/src/test/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorterSuite.java
---
@@ -70,14 +73,15 @@ public void testBasicSorting() throws
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211757342
**[Test build #56201 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56201/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211756345
**[Test build #56200 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56200/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211755392
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211755389
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211755381
**[Test build #56199 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56199/consoleFull)**
for PR 12490 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211754828
**[Test build #56199 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56199/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211715196
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211715186
**[Test build #56187 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56187/consoleFull)**
for PR 12490 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211715199
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12490#issuecomment-211714809
**[Test build #56187 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56187/consoleFull)**
for PR 12490 at commit
GitHub user ericl opened a pull request:
https://github.com/apache/spark/pull/12490
[SPARK-14724] Use radix sort for shuffles and sort operator when possible
## What changes were proposed in this pull request?
Spark currently uses TimSort for all in-memory sorts, including
88 matches
Mail list logo
