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

Proposal/rehash: Use async Messaging #416

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ android:
components:
- tools
- platform-tools
- android-26
- build-tools-26.0.2
- android-28
- build-tools-28.0.0

# as per http://blog.travis-ci.com/2014-12-17-faster-builds-with-container-based-infrastructure/
sudo: false
Expand Down
9 changes: 5 additions & 4 deletions build.gradle
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
subprojects {
allprojects {
buildscript {
repositories {
mavenCentral()
jcenter()
google()
}
dependencies {
classpath 'com.android.tools.build:gradle:3.0.1'
classpath 'com.android.tools.build:gradle:3.1.3'
classpath 'com.github.dcendents:android-maven-gradle-plugin:2.0'
classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.0'
classpath 'org.jfrog.buildinfo:build-info-extractor-gradle:4.5.2'
Expand All @@ -15,12 +15,13 @@ subprojects {

repositories {
mavenCentral()
google()
}
}

ext {
minSdkVersion = 9
compileSdkVersion = 26
buildToolsVersion = '26.0.2'
compileSdkVersion = 28
buildToolsVersion = '28.0.0'
sourceCompatibility = JavaVersion.VERSION_1_7
}
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-4.1-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.8.1-all.zip
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,21 @@
*/
package io.reactivex.android.schedulers;

import android.annotation.SuppressLint;
import android.os.Build;
import android.os.Handler;
import android.os.Looper;

import java.util.concurrent.Callable;

import android.os.Message;
import io.reactivex.Scheduler;
import io.reactivex.android.plugins.RxAndroidPlugins;
import java.util.concurrent.Callable;

/** Android-specific Schedulers. */
public final class AndroidSchedulers {

private static final class MainHolder {

static final Scheduler DEFAULT = new HandlerScheduler(new Handler(Looper.getMainLooper()));
static final Scheduler DEFAULT
= new HandlerScheduler(new Handler(Looper.getMainLooper()), false);
}

private static final Scheduler MAIN_THREAD = RxAndroidPlugins.initMainThreadScheduler(
Expand All @@ -43,8 +44,31 @@ public static Scheduler mainThread() {

/** A {@link Scheduler} which executes actions on {@code looper}. */
public static Scheduler from(Looper looper) {
return from(looper, false);
}

/**
* A {@link Scheduler} which executes actions on {@code looper}.
*
* @param async if true, the scheduler will use async messaging on API >= 16 to avoid VSYNC
* locking. On API < 16, this will no-op.
* @see android.os.Message#setAsynchronous(boolean)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't need to specify package since this is now imported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/
@SuppressLint("NewApi")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess because the conditional is reversed it is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah :/

public static Scheduler from(Looper looper, boolean async) {
if (looper == null) throw new NullPointerException("looper == null");
return new HandlerScheduler(new Handler(looper));
boolean useAsync = async && Build.VERSION.SDK_INT >= 16;
// Confirm the method is there
Handler handler = new Handler(looper);
if (useAsync) {
Message message = Message.obtain(handler);
try {
message.setAsynchronous(true);
} catch (NoSuchMethodError e) {
useAsync = false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should recycle the Message to be a good citizen!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
return new HandlerScheduler(new Handler(looper), useAsync);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You already created the Handler above. Can pass it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

derp, 6fbdff3

}

private AndroidSchedulers() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.reactivex.android.schedulers;

import android.annotation.SuppressLint;
import android.os.Handler;
import android.os.Message;
import io.reactivex.Scheduler;
Expand All @@ -23,9 +24,11 @@

final class HandlerScheduler extends Scheduler {
private final Handler handler;
private final boolean async;

HandlerScheduler(Handler handler) {
HandlerScheduler(Handler handler, boolean async) {
this.handler = handler;
this.async = async;
}

@Override
Expand All @@ -41,18 +44,21 @@ public Disposable scheduleDirect(Runnable run, long delay, TimeUnit unit) {

@Override
public Worker createWorker() {
return new HandlerWorker(handler);
return new HandlerWorker(handler, async);
}

private static final class HandlerWorker extends Worker {
private final Handler handler;
private final boolean async;

private volatile boolean disposed;

HandlerWorker(Handler handler) {
HandlerWorker(Handler handler, boolean async) {
this.handler = handler;
this.async = async;
}

@SuppressLint("NewApi") // the async flag is version-checked in its factory
@Override
public Disposable schedule(Runnable run, long delay, TimeUnit unit) {
if (run == null) throw new NullPointerException("run == null");
Expand All @@ -67,6 +73,9 @@ public Disposable schedule(Runnable run, long delay, TimeUnit unit) {
ScheduledRunnable scheduled = new ScheduledRunnable(handler, run);

Message message = Message.obtain(handler, scheduled);
if (async) {
message.setAsynchronous(true);
}
message.obj = this; // Used as token for batch disposal of this worker's runnables.

handler.sendMessageDelayed(message, unit.toMillis(delay));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@
import io.reactivex.functions.Consumer;
import io.reactivex.functions.Function;
import io.reactivex.plugins.RxJavaPlugins;
import java.util.Arrays;
import java.util.Collection;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.ParameterizedRobolectricTestRunner;
import org.robolectric.annotation.Config;
import org.robolectric.shadows.ShadowLooper;

Expand All @@ -46,9 +48,24 @@
import static org.robolectric.shadows.ShadowLooper.runUiThreadTasksIncludingDelayedTasks;
import static org.robolectric.shadows.ShadowLooper.unPauseMainLooper;

@RunWith(RobolectricTestRunner.class)
@Config(manifest=Config.NONE)
@RunWith(ParameterizedRobolectricTestRunner.class)
@Config(manifest=Config.NONE, sdk = 16)
public final class HandlerSchedulerTest {

@ParameterizedRobolectricTestRunner.Parameters(name = "async = {0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{true},
{false}
});
}

private Scheduler scheduler;

public HandlerSchedulerTest(boolean async) {
this.scheduler = new HandlerScheduler(new Handler(Looper.getMainLooper()), async);
}

@Before
public void setUp() {
RxJavaPlugins.reset();
Expand All @@ -61,8 +78,6 @@ public void tearDown() {
unPauseMainLooper();
}

private Scheduler scheduler = new HandlerScheduler(new Handler(Looper.getMainLooper()));

@Test
public void directScheduleOncePostsImmediately() {
CountingRunnable counter = new CountingRunnable();
Expand Down
2 changes: 1 addition & 1 deletion sample-app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ android {

defaultConfig {
minSdkVersion 15
targetSdkVersion 23
targetSdkVersion 28
versionCode 1
versionName '1.0'
}
Expand Down