-
Notifications
You must be signed in to change notification settings - Fork 3k
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(mergeMapTo): remove tryCatch/errorObject (~2x improvement) #1302
Conversation
try { | ||
result = resultSelector(outerValue, innerValue, outerIndex, innerIndex); | ||
} catch (err) { | ||
destination.error(err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be sure to return
after this line. It shouldn't continue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops.
4432922
to
8fb00ce
Compare
@Blesh fixed. |
Results on a Intel Core i7-3770 (Ivy Bridge) running Ubuntu: | RxJS 4.0.7 | RxJS 5.0.0-beta.1 | factor | % improved ------------------------------------------------------------------------------------------------- mergemapto-resultselector - immediate | 811 (±1.50%) | 5,376 (±1.10%) | 6.63x | 562.7% mergemapto-resultselector | 369 (±0.43%) | 1,957 (±0.45%) | 5.30x | 429.7%
Before: | RxJS 4.0.7 | RxJS 5.0.0-beta.1 | factor | % improved ------------------------------------------------------------------------------------------------- mergemapto-resultselector - immediate | 811 (±1.50%) | 5,376 (±1.10%) | 6.63x | 562.7% mergemapto-scalar - immediate | 6,110 (±0.45%) | 113,487 (±0.91%) | 18.58x | 1,757.5% mergemapto - immediate | 1,439 (±0.95%) | 17,017 (±0.29%) | 11.83x | 1,082.8% mergemapto-resultselector | 369 (±0.43%) | 1,957 (±0.45%) | 5.30x | 429.7% mergemapto-scalar | 3,740 (±0.45%) | 66,278 (±1.18%) | 17.72x | 1,672.3% mergemapto | 490 (±0.39%) | 3,685 (±0.44%) | 7.52x | 651.7% After: | RxJS 4.0.7 | RxJS 5.0.0-beta.1 | factor | % improved ------------------------------------------------------------------------------------------------- mergemapto-resultselector - immediate | 810 (±1.70%) | 11,428 (±3.41%) | 14.11x | 1,310.6% mergemapto-scalar - immediate | 6,401 (±2.48%) | 117,259 (±2.09%) | 18.32x | 1,731.9% mergemapto - immediate | 1,452 (±0.44%) | 17,406 (±0.35%) | 11.99x | 1,098.9% mergemapto-resultselector | 383 (±1.24%) | 3,288 (±0.42%) | 8.58x | 757.7% mergemapto-scalar | 3,837 (±0.57%) | 62,294 (±3.05%) | 16.23x | 1,523.3% mergemapto | 525 (±2.37%) | 3,795 (±0.79%) | 7.23x | 622.6%
8fb00ce
to
c0b1ec3
Compare
LGTM |
Merged with 42bcced, thanks @luisgabriel |
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. |
Before:
After: