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

use Iterator#size in RDD#count #736

Closed
wants to merge 1 commit into from
Closed

Conversation

cloud-fan
Copy link
Contributor

in RDD#count, we used while loop to get the size of Iterator because that Iterator#size used a for loop, which was slightly slower in that version of Scala. But for now, the current version of scala will translate the for loop in Iterator#size into foreach, which uses while loop to iterate the Iterator. So we can use Iterator#size directly now.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@rxin
Copy link
Contributor

rxin commented May 13, 2014

Thanks for submitting this. Do you have a link to the Scala compiler/collection library ticket that impacted this?

@rxin
Copy link
Contributor

rxin commented May 13, 2014

Jenkins, test this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14931/

@cloud-fan
Copy link
Contributor Author

@rxin I'm sorry I didn't got a link for that, but I didn't find any discussion about performance issue of Iterator#size, either. I just checked the source code of Iterator and desugar scala for loop to see what happened.
scala -Xprint:parser -e "var count = 0;for (i <- List(1,2,3).iterator) count += 1"
You can try it yourself, it will be translated into var count = 0;List(1, 2, 3).iterator.foreach( i => count += 1)
And the Iterator#foreach implementation is { while (hasNext) f(next()) }, it's same with what spark do to calculate the size of iterator.
Anyway, get size of an iterator is a basic function, it is logical to let the language lib do it.

@mridulm
Copy link
Contributor

mridulm commented May 13, 2014

This is not equivalent performance wise from casual look.
Even assuming everything is same, it is still invoking function in loop versus direct addition.

@cloud-fan
Copy link
Contributor Author

I wrote a simple benchmark to test performance, Iterator#size really sucks... Sorry for my mistake, I'll close this pull request :(

@cloud-fan cloud-fan closed this May 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants