-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Re-submission of CachedThreadScheduler #1276
Re-submission of CachedThreadScheduler #1276
Conversation
RxJava-pull-requests #1174 SUCCESS |
import java.util.concurrent.*; | ||
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; | ||
|
||
/* package */class CachedThreadScheduler extends Scheduler { |
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.
Could you add final
here?
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.
Sure, wasn't sure how it should be, since there doesn't seem to be any standard to follow there. ImmediateScheduler
is final
, but not EventLoopsScheduler
or NewThreadScheduler
.
Those should be final as well. I sometimes add them in one PR, forget to add them in another and at the end, a third PR gets merged without it. I plan to do another cleanup round before 1.0 so hopefully |
RxJava-pull-requests #1175 SUCCESS |
Testing with this code: public static void main(String[] args) {
Observable.timer(0, 10, TimeUnit.MILLISECONDS).take(100).flatMap(i -> {
return Observable.just(i).subscribeOn(Schedulers.io()).map(t -> {
try {
Thread.sleep(10);
} catch (Exception e) {
e.printStackTrace();
}
return t + "-" + Thread.currentThread();
});
}).toBlocking().forEach(System.out::println );
} Without this pull request it creates 100 threads:
With this pull request it uses only 2 threads:
|
Thanks @jbripley for this! |
Re-submission of CachedThreadScheduler
Thanks for all the pointers @akarnokd. I've learned a lot about both thread safe Java and how the |
You are welcome. |
Merged in the latest from the master branch and refactored some internals in CachedThreadScheduler to better match the latest
Scheduler
changes. Aimed at solving #1140