[SPARK-2983] [PySpark] improve performance of sortByKey() #1898
Conversation
1. skip partitionBy() when numOfPartition is 1 2. use bisect_left (O(lg(N))) instread of loop (O(N)) in rangePartitioner
|
QA tests have started for PR 1898. This patch merges cleanly. |
|
QA results for PR 1898: |
davies
changed the title
|
QA tests have started for PR 1898. This patch merges cleanly. |
|
QA results for PR 1898: |
There was a problem hiding this comment.
Why did we have the flatMap(lambda x: x) before? Just want to make sure we're not removing something useful.
There was a problem hiding this comment.
Ah I guess it's due to the yield -> return above?
There was a problem hiding this comment.
Yes, but I have no idea why it's done in this way. I think it's not necessary.
There was a problem hiding this comment.
Got it, yeah. It seems unnecessary.
There was a problem hiding this comment.
I think there might be two unintended side effects of this change. This code used to work in pyspark:
sc.parallelize([5,3,4,2,1]).map(lambda x: (x,x)).sortByKey().take(1)
Now it failswith the error:
File "<...>/spark/python/pyspark/rdd.py", line 1023, in takeUpToNumLeft
yield next(iterator)
TypeError: list object is not an iterator
Changing mapFunc and sort back to generators rather than regular functions fixes that problem.
After making that change, there is a second side effect due to the removal of flatMap where the above code returns the following unexpected result due to the default partitioning scheme:
[[(1, 1), (2, 2)]]
Removing sortByKey, e.g.:
sc.parallelize([5,3,4,2,1]).map(lambda x: (x,x)).take(1)
returns the expected result [(5, 5)]. Restoring the call to flatMap resolves this as well.
There was a problem hiding this comment.
Thanks for pointing this out.. sounds like we should look into this.
There was a problem hiding this comment.
@str-janus thanks, this will be fixed in #2045
|
Looks good to me; I'll merge it. |
|
Merged into 1.1. |
1. skip partitionBy() when numOfPartition is 1 2. use bisect_left (O(lg(N))) instread of loop (O(N)) in rangePartitioner Author: Davies Liu <[email protected]> Closes #1898 from davies/sort and squashes the following commits: 0a9608b [Davies Liu] Merge branch 'master' into sort 1cf9565 [Davies Liu] improve performance of sortByKey() (cherry picked from commit 434bea1) Signed-off-by: Matei Zaharia <[email protected]>
1. skip partitionBy() when numOfPartition is 1 2. use bisect_left (O(lg(N))) instread of loop (O(N)) in rangePartitioner Author: Davies Liu <[email protected]> Closes apache#1898 from davies/sort and squashes the following commits: 0a9608b [Davies Liu] Merge branch 'master' into sort 1cf9565 [Davies Liu] improve performance of sortByKey()
4 participants
rangePartitioner