Skip to content

[SPARK-31175][SQL] Avoid creating reverse comparator for each compare in InterpretedOrdering#27938

Closed
Ngone51 wants to merge 1 commit intoapache:masterfrom
Ngone51:reverse_comparator
Closed

[SPARK-31175][SQL] Avoid creating reverse comparator for each compare in InterpretedOrdering#27938
Ngone51 wants to merge 1 commit intoapache:masterfrom
Ngone51:reverse_comparator

Conversation

@Ngone51
Copy link
Member

@Ngone51 Ngone51 commented Mar 17, 2020

What changes were proposed in this pull request?

Prpend - to the compare result instead of creating a new reverse comparator for each compare when sorting in DESC order in InterpretedOrdering.

Why are the changes needed?

Currently, we'll create a new reverse comparator for each compare in InterpretedOrdering, which could generate lots of small and instant object and hurt JVM when there're plenty of data.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass Jenkins.

@SparkQA
Copy link

SparkQA commented Mar 17, 2020

Test build #119939 has finished for PR 27938 at commit 6f759b4.

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

@Ngone51
Copy link
Member Author

Ngone51 commented Mar 18, 2020

ping @cloud-fan @HyukjinKwon Please take a look, thanks!

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 8bfaa62 Mar 18, 2020
@Ngone51
Copy link
Member Author

Ngone51 commented Mar 18, 2020

thanks all!

@dongjoon-hyun
Copy link
Member

+1, late LGTM.

sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
… in InterpretedOrdering

### What changes were proposed in this pull request?

Prpend `-` to the compare result instead of creating a new reverse comparator for each compare when sorting in DESC order in InterpretedOrdering.

### Why are the changes needed?

Currently, we'll create a new reverse comparator for each compare in InterpretedOrdering, which could generate lots of small and instant object and hurt JVM when there're plenty of data.

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

Pass Jenkins.

Closes apache#27938 from Ngone51/reverse_comparator.

Authored-by: yi.wu <yi.wu@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments