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

Convert to scan to use lift #790

Merged
merged 1 commit into from
Feb 12, 2014
Merged

Conversation

abersnaze
Copy link
Contributor

I switched the scan to an operator and cleaned up what looked like a lot of unnecessary code from the summing operations.

@cloudbees-pull-request-builder

RxJava-pull-requests #711 FAILURE
Looks like there's a problem with this pull request

@abersnaze
Copy link
Contributor Author

not sure what went wrong on cloud bees because it works locally from the command line and eclipse.

@cloudbees-pull-request-builder

RxJava-pull-requests #714 FAILURE
Looks like there's a problem with this pull request

@@ -63,244 +62,39 @@ public Double call(Double accu, Double next) {
});
}

/**
* Compute the sum by extracting integer values from the source via an
Copy link
Member

Choose a reason for hiding this comment

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

What is the performance impact of using scan instead of custom implementations for sums?

Despite it being elegant, the machinery in scan far outweighs sum += i so I'd like to know the performance hit.

@cloudbees-pull-request-builder

RxJava-pull-requests #786 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

RxJava-pull-requests #794 SUCCESS
This pull request looks good

@abersnaze
Copy link
Contributor Author

To answer @benjchristensen's question about the performance of sumInteger/Float/Long/Double.

Originally half of the implementations were based on reduce(R, Func2<R, T, R) and the other half were custom. I just removed the custom code for reduce(Func2<T, T, T>) which has the behavior of throwing an error when the Observable is empty.

I'm having a hard time imagining a critical situation for sum together an Observable numbers to justify so much custom code.

benjchristensen added a commit that referenced this pull request Feb 12, 2014
Convert to scan to use lift
@benjchristensen benjchristensen merged commit 6c578f7 into ReactiveX:master Feb 12, 2014
@abersnaze abersnaze deleted the scan branch February 12, 2014 20:55
jihoonson pushed a commit to jihoonson/RxJava that referenced this pull request Mar 6, 2020
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.

None yet

3 participants