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

2.x: Should schedulers have RxThreadFactory constructor parameters? #4993

Closed
ZacSweers opened this issue Jan 13, 2017 · 14 comments
Closed

2.x: Should schedulers have RxThreadFactory constructor parameters? #4993

ZacSweers opened this issue Jan 13, 2017 · 14 comments

Comments

@ZacSweers
Copy link
Contributor

RxJava 1 allowed for specifying a custom thread factory, but it looks like this feature did not make it to RxJava 2. Was this intentional or something that a PR would be welcome for?

@akarnokd
Copy link
Member

Which scheduler? Computation? It was intentional. You can now configure the priority of each scheduler so there was little benefit from a customizing the factory. Use the ParallelScheduler from extensions which allows more customization.

@ZacSweers
Copy link
Contributor Author

We used it for the IO scheduler as well. It was useful for tracking our own thread naming and setting their priority to android-specific priorities. We'd like to use it for that and SingleScheduler. Can ParallelScheduler be used as a substitute for non-computation schedulers too?

@akarnokd
Copy link
Member

Since scheduler implementations are internal, it is not recommended anyway to use them directly.

ParallelScheduler is an individual scheduler with a specified number of threads that doesn't change over time. So unlike io(), ParallelScheduler won't stop its threads when they become idle for too long.

@ZacSweers
Copy link
Contributor Author

Since scheduler implementations are internal, it is not recommended anyway to use them directly.

I'm not sure I follow. As in we shouldn't create a new IoScheduler instance?

@ZacSweers
Copy link
Contributor Author

Prior discussions/work for reference - #3724 #3879

@akarnokd
Copy link
Member

Anything in the io.reactivex.internal.* is considered private and not part of any binary or API compatibility contracts.

@akarnokd
Copy link
Member

So either use Schedulers.from(Executor) with your own pool or copy the internal code and change it locally.

@ZacSweers
Copy link
Contributor Author

I missed that they were internal now. That's disappointing to see, and seems like a bit of a step backward compared to RxJava 1. We'll just copy then :|

@JakeWharton
Copy link
Contributor

JakeWharton commented Jan 13, 2017 via email

@ZacSweers
Copy link
Contributor Author

Yeah I was referring to the factory methods. Basically wanted to do the same thing you added in the linked PR above

@ZacSweers ZacSweers reopened this Jan 14, 2017
@ZacSweers
Copy link
Contributor Author

ZacSweers commented Jan 14, 2017

Those could easily be replicated on 2.x.

@akarnokd any thoughts on this? This is what I was thinking of in opening this issue, in case I wasn't clear before

@akarnokd
Copy link
Member

They will be still internal classes. However, we could expose them, for example, Schedulers.newComputation(), Schedulers.newIO() etc where extra parameters can be passed in, similar to how Project Reactor's schedulers were exposed.

@ZacSweers
Copy link
Contributor Author

ZacSweers commented Jan 14, 2017

That sounds reasonable to me. The APIs they expose cover pretty much everything we'd want, would be happy to make a PR matching those if you're up for it.

@akarnokd
Copy link
Member

Sure.

ZacSweers added a commit to ZacSweers/RxJava that referenced this issue Jan 18, 2017
Resolves ReactiveX#4993

This is a pretty vanilla copy from RxJava 1's implementation. Note that I had to tune NewThread scheduler to not be a singleton to support this.

We had talked about borrowing from project reactor's APIs for different overloads, let me know if you think we should add more fine-grained controls through these.
akarnokd pushed a commit that referenced this issue Jan 25, 2017
* Add scheduler creation factories

Resolves #4993

This is a pretty vanilla copy from RxJava 1's implementation. Note that I had to tune NewThread scheduler to not be a singleton to support this.

We had talked about borrowing from project reactor's APIs for different overloads, let me know if you think we should add more fine-grained controls through these.

* Add `@since` info

* Change failure string to "is null"

* Move to RxJavaPlugins

* Remove no-arg overloads

* Rename to make it clearer about creation

Added scheduler because we're not in Scheduler anymore. Changed to "create" because "newNewThread" was weird

* Add tests (WIP)

* Remove unnecessary nullcheck

* Remove double try

* Fix tests, make them more robust with integration flow

* Shut down custom schedulers when done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants