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-22857] Optimize code by inspecting code #20044

Closed
wants to merge 2 commits into from

Conversation

xubo245
Copy link
Contributor

@xubo245 xubo245 commented Dec 21, 2017

Optimize code by inspecting code, including:
remove some unused import
change invoking array size method to length method
use head method

What changes were proposed in this pull request?

Optimize code by inspecting code

How was this patch tested?

No

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen
Copy link
Member

srowen commented Dec 21, 2017

In the past we've said 'no' to random small changes to the code like this. Some changes like _ to null probably aren't worth it at all. (0) to .head is the same, I think.

Removing unused imports is OK, and I personally am OK with .size to .length for arrays because it avoids an implicit conversion. But, it needs to be done consistently or not at all.

Optimize code by inspecting code, including:
remove some unused import
change invoking array size method to length method
@xubo245
Copy link
Contributor Author

xubo245 commented Dec 21, 2017

Ok, I remove some changes.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do all or nothing. I am sure there are more array issues at least

@xubo245
Copy link
Contributor Author

xubo245 commented Dec 25, 2017

@srowen I find all related array size warning,
4

are there any other issues?

@srowen
Copy link
Member

srowen commented Dec 26, 2017

There are lots more than this, no. I don't think it's worthwhile, unless you're finding several that might cause a performance problem.

srowen added a commit to srowen/spark that referenced this pull request Dec 31, 2017
@srowen srowen mentioned this pull request Dec 31, 2017
@asfgit asfgit closed this in f5b7714 Jan 1, 2018
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.

3 participants