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

Unnecessary dependency on Schedulers.trampoline() for non-time-based refcount operator #6451

Closed
mycallmax opened this issue Apr 3, 2019 · 1 comment

Comments

@mycallmax
Copy link

In the constructor of ObservableRefCount that takes ConnectableObservable<T> source as the argument, we set timeout to 0L. In that specific use case of ObservableRefCount, scheduler is never needed. It's only referenced in cancel() method but if timeout is 0, it won't be triggered at all because there is early return. I am wondering if we could avoid referencing Schedulers.trampoline() in that specific use case of ObservableRefCount to keep the dependency clear.

public ObservableRefCount(ConnectableObservable<T> source) {
this(source, 1, 0L, TimeUnit.NANOSECONDS, Schedulers.trampoline());
}

void cancel(RefConnection rc) {
SequentialDisposable sd;
synchronized (this) {
if (connection == null || connection != rc) {
return;
}
long c = rc.subscriberCount - 1;
rc.subscriberCount = c;
if (c != 0L || !rc.connected) {
return;
}
if (timeout == 0L) {
timeout(rc);
return;
}
sd = new SequentialDisposable();
rc.timer = sd;
}
sd.replace(scheduler.scheduleDirect(rc, timeout, unit));
}

The reasons are the following:

  1. In projects that don't depend on Schedulers class, if there is no reference to Schedulers, the whole Schedulers can be stripped out of the library after optimizations (e.g., proguard). With constructor that references Schedulers, the optimizer can't properly strip it out. In our quick test of our Android app, we were able to reduce the RxJava library size dependency from 51KB to 37KB (after optimization but before compression) by simply avoiding access to Schedulers in ObservableRefCount.
  2. In terms of modularity, ObservableRefCount is just an operator so it by itself should probably not have dependency on what available pool of schedulers (Schedulers) there are. It should just know that there is some Scheduler that could be passed to ObservableRefCount when ObservableRefCount needs it.
@akarnokd
Copy link
Member

akarnokd commented Apr 3, 2019

Yes, it can be replaced by null. PR welcome.

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

2 participants