Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-19991][CORE][YARN] FileSegmentManagedBuffer performance improvement #17567

Closed
wants to merge 1 commit into from

Conversation

srowen
Copy link
Member

@srowen srowen commented Apr 7, 2017

What changes were proposed in this pull request?

Avoid NoSuchElementException every time ConfigProvider.get(val, default) falls back to default. This apparently causes non-trivial overhead in at least one path, and can easily be avoided.

See #17329

How was this patch tested?

Existing tests

@srowen
Copy link
Member Author

srowen commented Apr 7, 2017

CC @witgo @vanzin

@SparkQA
Copy link

SparkQA commented Apr 7, 2017

Test build #75609 has finished for PR 17567 at commit 3828d03.

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

@vanzin
Copy link
Contributor

vanzin commented Apr 7, 2017

LGTM.

@SparkQA
Copy link

SparkQA commented Apr 7, 2017

Test build #3646 has finished for PR 17567 at commit 3828d03.

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

@witgo
Copy link
Contributor

witgo commented Apr 8, 2017

LGTM.
Are there any performance test reports?

@srowen
Copy link
Member Author

srowen commented Apr 8, 2017

I don't have a benchmark. I was going by your report that this was a hotspot. It is a simple enough change that it seemed OK to make even with minimal runtime benefit.

@witgo
Copy link
Contributor

witgo commented Apr 8, 2017

OK, I see.

@srowen
Copy link
Member Author

srowen commented Apr 9, 2017

Merged to master

@asfgit asfgit closed this in 1f0de3c Apr 9, 2017
@srowen srowen deleted the SPARK-19991 branch April 11, 2017 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants