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

Perf or no perf, there is no try... #1251

Merged
merged 21 commits into from
Jan 27, 2016
Merged

Perf or no perf, there is no try... #1251

merged 21 commits into from
Jan 27, 2016

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Jan 26, 2016

I discovered today that adding custom tryCatcher members to operators with try/catch optimization concerns actually improved performance quite a bit.

So far: map doubled in overall speed (from factor of 1.7x to 3.2x on my machine),
mergeMap improved by a "factor" of a little more than 1x
zip improved by a "factor" of a little more than 1x-2x.

("factor" coming from micro perf output).

This will be part of an effort to go through and examine improvements that can be made where tryCatch/errorObject is being used.

See commits for details.

All of these perf tests are of course in V8/Node, however I don't see any reason they wouldn't carry over to other JIT'ed runtimes.

cc/ @trxcllnt

}
}

_notifyResultSelector(outerValue: T, innerValue: R, outerIndex: number, innerIndex: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

private?

@luisgabriel
Copy link
Contributor

👍

@kwonoj
Copy link
Member

kwonoj commented Jan 26, 2016

Cool. How do we determine other operators can apply same pattern? try & compare perf manually?

@benlesh
Copy link
Member Author

benlesh commented Jan 26, 2016

Yes, I'm trying and comparing perf manually

@benlesh benlesh merged commit 7004c26 into ReactiveX:master Jan 27, 2016
@benlesh
Copy link
Member Author

benlesh commented Jan 27, 2016

Merging this work for now, it's about half done, but it gives a lot of examples of how to improve perf by using tryCatch handling that's a little more focused than tryCatch/errorObject

@benlesh
Copy link
Member Author

benlesh commented Jan 27, 2016

Eventually, tryCatch and errorObject can probably be removed from this project entirely.

@kwonoj
Copy link
Member

kwonoj commented Jan 27, 2016

I'll try to apply same approach on other operators as well.

Eventually, tryCatch and errorObject can probably be removed from this project entirely.

: makes sense - in that case if refactoring does not brings noticeable perf improvement, will it still considered as good to checked in by removing usage of custom tryCatcher?

@tetsuharuohzeki
Copy link
Contributor

@Blesh

All of these perf tests are of course in V8/Node, however I don't see any reason they wouldn't carry over to other JIT'ed runtimes.

How about ChakraNode (https://github.com/Microsoft/node)? We may be able to use it as a sample.

@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants