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

Subjects fixes performance improvements #652

Merged
merged 6 commits into from
Dec 23, 2013

Conversation

akarnokd
Copy link
Member

Based on PR #651

Improvements upon Ben's excellent work:

  • (Original performance for me was 1.662.152 ops/sec on average.)
  • Storing subscriptions and observers in arrays rather than in a Map inside SubjectSubscriptionManager.. This adds +2.340.000 ops/sec on average; total ~ 4.000.000 ops/sec so far.
  • SafeObserver uses a volatile for the actual Observer which is swapped to a nopObserver if a terminal case occurs. This avoids an atomic get and if statement and adds a +100.000 ops/second on average; total ~4.100.000 ops/sec so far.
  • I suggest adding a create option to set the initial buffer size (not done here) which would reduce the number of array-resizes. With 2048 as a sweet spot on my machine, this adds +1.100.000 ops/second; total 5.200.000 so far.

Bounded version
We have a boundable ReplaySubject in main named CustomReplaySubject and PR #649 to improve a bit upon its performance. It might benefit from the changes above as well. It would be more tricky implementation in terms of the replay operation, since the buffer would be in constant virtual index motion (including its start and end), so observers would need to atomically get a value from it whose index could get below the virtual start index of the buffer. Will think about this.

benjchristensen and others added 4 commits December 21, 2013 20:37
- Common logic composed inside SubjectSubscriptionManager
- ReplaySubject does not block while replaying to new subscribers
- Added unit tests and fixed behavior while reviewing with @headinthebox compared to Rx.Net
- Uses mostly non-blocking approach (I believe it’s all correct, unit and long running tests have been used to prove it. The tests found concurrency problems during development and became stable once I got the design correct. As with all concurrent code I may be missing something.)
- previous commit got non-blocking working but perf tests showed it slow
- this commit retains non-blocking but surpasses master branch performance

Master branch: 11,947,459 ops/sec
This commit: 16,151,174 ops/sec
@cloudbees-pull-request-builder

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

@cloudbees-pull-request-builder

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

@benjchristensen
Copy link
Member

Wow ... nice performance improvements!

     * ArrayList with raw values & non-blocking (no synchronization)
     * 
     * Run: 10 - 16,069,678 ops/sec
     * Run: 11 - 15,954,688 ops/sec
     * Run: 12 - 16,158,874 ops/sec
     * Run: 13 - 16,209,504 ops/sec
     * Run: 14 - 16,151,174 ops/sec
     * 
     * Map to Arrays and other enhancements from https://github.com/Netflix/RxJava/pull/652
     * 
     * Run: 10 - 54,231,405 ops/sec
     * Run: 11 - 56,239,490 ops/sec
     * Run: 12 - 55,424,384 ops/sec
     * Run: 13 - 56,370,421 ops/sec
     * Run: 14 - 56,617,767 ops/sec

@benjchristensen
Copy link
Member

I'm going to revert the SafeObserver changes as they are trivial improvements at best:

Old SafeObserver

Run: 0 - 3,751,704 ops/sec 
Run: 1 - 34,619,086 ops/sec 
Run: 2 - 30,483,715 ops/sec 
Run: 3 - 27,746,023 ops/sec 
Run: 4 - 54,078,608 ops/sec 
Run: 5 - 55,107,045 ops/sec 
Run: 6 - 53,935,396 ops/sec 
Run: 7 - 54,947,168 ops/sec 
Run: 8 - 57,024,246 ops/sec 
Run: 9 - 55,059,712 ops/sec 
Run: 10 - 56,904,832 ops/sec 
Run: 11 - 55,919,967 ops/sec 
Run: 12 - 55,076,087 ops/sec 
Run: 13 - 55,066,685 ops/sec 
Run: 14 - 55,025,476 ops/sec 

Run: 0 - 3,839,266 ops/sec 
Run: 1 - 34,115,371 ops/sec 
Run: 2 - 29,675,175 ops/sec 
Run: 3 - 28,677,042 ops/sec 
Run: 4 - 55,405,652 ops/sec 
Run: 5 - 55,260,220 ops/sec 
Run: 6 - 55,147,464 ops/sec 
Run: 7 - 54,261,126 ops/sec 
Run: 8 - 53,941,505 ops/sec 
Run: 9 - 54,324,501 ops/sec 
Run: 10 - 55,125,576 ops/sec 
Run: 11 - 56,102,870 ops/sec 
Run: 12 - 55,061,834 ops/sec 
Run: 13 - 55,476,039 ops/sec 
Run: 14 - 55,073,054 ops/sec 

Run: 0 - 3,704,536 ops/sec 
Run: 1 - 34,694,514 ops/sec 
Run: 2 - 30,778,227 ops/sec 
Run: 3 - 28,441,329 ops/sec 
Run: 4 - 54,116,946 ops/sec 
Run: 5 - 55,204,699 ops/sec 
Run: 6 - 54,859,450 ops/sec 
Run: 7 - 55,214,757 ops/sec 
Run: 8 - 55,005,500 ops/sec 
Run: 9 - 55,339,118 ops/sec 
Run: 10 - 55,501,903 ops/sec 
Run: 11 - 55,074,570 ops/sec 
Run: 12 - 55,102,187 ops/sec 
Run: 13 - 55,756,278 ops/sec 
Run: 14 - 54,768,411 ops/sec 


New SafeObserver


Run: 0 - 3,983,308 ops/sec 
Run: 1 - 34,767,250 ops/sec 
Run: 2 - 30,806,957 ops/sec 
Run: 3 - 29,855,113 ops/sec 
Run: 4 - 57,451,453 ops/sec 
Run: 5 - 55,515,152 ops/sec 
Run: 6 - 56,086,822 ops/sec 
Run: 7 - 56,295,529 ops/sec 
Run: 8 - 55,371,905 ops/sec 
Run: 9 - 55,816,653 ops/sec 
Run: 10 - 55,793,296 ops/sec 
Run: 11 - 56,011,426 ops/sec 
Run: 12 - 55,568,521 ops/sec 
Run: 13 - 55,396,137 ops/sec 
Run: 14 - 56,353,267 ops/sec 


Run: 0 - 3,933,367 ops/sec 
Run: 1 - 34,498,342 ops/sec 
Run: 2 - 30,233,584 ops/sec 
Run: 3 - 29,179,785 ops/sec 
Run: 4 - 55,761,874 ops/sec 
Run: 5 - 55,948,124 ops/sec 
Run: 6 - 55,264,801 ops/sec 
Run: 7 - 56,267,020 ops/sec 
Run: 8 - 57,474,567 ops/sec 
Run: 9 - 55,879,657 ops/sec 
Run: 10 - 55,998,880 ops/sec 
Run: 11 - 56,044,073 ops/sec 
Run: 12 - 55,498,515 ops/sec 
Run: 13 - 56,204,720 ops/sec 
Run: 14 - 55,845,954 ops/sec 


Run: 0 - 3,981,914 ops/sec 
Run: 1 - 34,160,822 ops/sec 
Run: 2 - 30,873,631 ops/sec 
Run: 3 - 29,135,067 ops/sec 
Run: 4 - 55,845,330 ops/sec 
Run: 5 - 55,101,883 ops/sec 
Run: 6 - 55,724,276 ops/sec 
Run: 7 - 56,085,564 ops/sec 
Run: 8 - 55,639,942 ops/sec 
Run: 9 - 56,464,955 ops/sec 
Run: 10 - 55,453,275 ops/sec 
Run: 11 - 56,115,463 ops/sec 
Run: 12 - 56,509,945 ops/sec 
Run: 13 - 53,863,348 ops/sec 
Run: 14 - 55,866,858 ops/sec 

The rest of the changes are awesome, thank you!

@benjchristensen benjchristensen merged commit 6d11a55 into ReactiveX:master Dec 23, 2013
@akarnokd akarnokd deleted the subjects-fixes branch January 13, 2014 09:58
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