From c201130a227ad33403e9222ec4a5eb345d30c5f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Thu, 18 Apr 2013 10:30:46 +0200 Subject: [PATCH 1/5] Immediate and delayed executors unification A ScheduledExecutorService *is* an Executor, so we can use the same to avoids to create too many threads. The execute(runnable) method becomes a particular case of execute(runnable, delay), with delay=0. As the user can change the executor, delayed executions are not supported if the given executor does not support scheduling. This unification paves the way for adding serial execution feature. --- .../api/BackgroundExecutor.java | 52 +++++++++++++++---- .../processing/BackgroundProcessor.java | 6 +-- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java index 35b90a96aa..19a1e67f1f 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -22,22 +22,54 @@ public class BackgroundExecutor { - private static Executor executor = Executors.newCachedThreadPool(); - private static ScheduledExecutorService scheduledExecutor = Executors.newScheduledThreadPool(2 * Runtime.getRuntime().availableProcessors()); + private static Executor executor = Executors.newScheduledThreadPool(2 * Runtime.getRuntime().availableProcessors()); + /** + * Execute a task after the given delay. + * + * @param runnable + * the task to execute + * @param delay + * the time from now to delay execution, in milliseconds + * @throws IllegalArgumentException + * if delay is strictly positive and the current + * executor does not support scheduling (if + * {@link #setExecutor(Executor)} has been called with such an + * executor) + */ + public static void execute(Runnable runnable, long delay) { + if (delay > 0) { + if (!(executor instanceof ScheduledExecutorService)) { + throw new IllegalArgumentException("The executor set does not support scheduling"); + } + ((ScheduledExecutorService) executor).schedule(runnable, delay, TimeUnit.MILLISECONDS); + } else { + /* execute now */ + executor.execute(runnable); + } + } + + /** + * Execute a task. + * + * @param runnable + * the task to execute + */ public static void execute(Runnable runnable) { - executor.execute(runnable); + execute(runnable, 0); } + /** + * Change the executor. + * + * Note that if the given executor is not a {@link ScheduledExecutorService} + * then executing a task after a delay will not be supported anymore. + * + * @param executor + * the new executor + */ public static void setExecutor(Executor executor) { BackgroundExecutor.executor = executor; } - public static void executeDelayed(Runnable runnable, long delay) { - scheduledExecutor.schedule(runnable, delay, TimeUnit.MILLISECONDS); - } - - public static void setScheduledExecutor(ScheduledExecutorService scheduledExecutor) { - BackgroundExecutor.scheduledExecutor = scheduledExecutor; - } } diff --git a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java index 9db4750e47..130e47f328 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java @@ -60,11 +60,7 @@ public void process(Element element, JCodeModel codeModel, EBeanHolder holder) t JClass backgroundExecutorClass = holder.refClass(BackgroundExecutor.class); JInvocation executeCall; - if (delay == 0) { - executeCall = backgroundExecutorClass.staticInvoke("execute").arg(_new(anonymousRunnableClass)); - } else { - executeCall = backgroundExecutorClass.staticInvoke("executeDelayed").arg(_new(anonymousRunnableClass)).arg(lit(delay)); - } + executeCall = backgroundExecutorClass.staticInvoke("execute").arg(_new(anonymousRunnableClass)).arg(lit(delay)); delegatingMethod.body().add(executeCall); From 879f196e4fcfd647ad0b674827ca22ef51459953 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Thu, 18 Apr 2013 10:24:51 +0200 Subject: [PATCH 2/5] Serial execution in background feature Adds an optional serial attribute to @Background: @Background(serial="some_id") This idea comes from #309: https://github.com/excilys/androidannotations/issues/309 The principle of this implementation is to keep a separate queue for each serial identifier, and to give each task to the (unique) executor only after the previous task with the same serial identifier (if any) has completed execution. It guarantees that all tasks with the same serial identifier will be called in the order, one at a time (but not necessarily on the same thread). It is still compatible with the delay attribute. For example: @Background(serial="some_id", delay=2000) executes the task in background after at least 2 seconds *and* after the previous task requested with serial="some_id" (if any) have completed execution. --- .../annotations/Background.java | 4 +- .../api/BackgroundExecutor.java | 156 +++++++++++++++++- .../processing/BackgroundProcessor.java | 5 +- 3 files changed, 153 insertions(+), 12 deletions(-) diff --git a/AndroidAnnotations/androidannotations-api/src/main/java/org/androidannotations/annotations/Background.java b/AndroidAnnotations/androidannotations-api/src/main/java/org/androidannotations/annotations/Background.java index 323b489223..7007d3c7fa 100644 --- a/AndroidAnnotations/androidannotations-api/src/main/java/org/androidannotations/annotations/Background.java +++ b/AndroidAnnotations/androidannotations-api/src/main/java/org/androidannotations/annotations/Background.java @@ -28,5 +28,7 @@ @Retention(RetentionPolicy.CLASS) @Target(ElementType.METHOD) public @interface Background { - long delay() default 0; + int delay() default 0; /* in milliseconds */ + + String serial() default ""; } diff --git a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java index 19a1e67f1f..b5050519ed 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -15,6 +15,12 @@ */ package org.androidannotations.api; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -24,39 +30,121 @@ public class BackgroundExecutor { private static Executor executor = Executors.newScheduledThreadPool(2 * Runtime.getRuntime().availableProcessors()); + /* + * serialRunning is used as a lock in synchronized blocks for both + * serialRunning and serialQueues access + */ + + /* Set of queueIds having a currently running task */ + private static final Set serialRunning = new HashSet(); + + /* Tasks queues for each serial */ + private static final Map> serialQueues = new HashMap>(); + /** - * Execute a task after the given delay. + * Execute a task after (at least) the given delay and + * after all tasks added with the same non-null serial (if any) + * have completed execution. * * @param runnable * the task to execute * @param delay * the time from now to delay execution, in milliseconds + * @param serial + * the serial queue to use (null or "" + * for no serial execution) * @throws IllegalArgumentException * if delay is strictly positive and the current * executor does not support scheduling (if * {@link #setExecutor(Executor)} has been called with such an * executor) */ - public static void execute(Runnable runnable, long delay) { - if (delay > 0) { - if (!(executor instanceof ScheduledExecutorService)) { - throw new IllegalArgumentException("The executor set does not support scheduling"); + public static void execute(Runnable runnable, int delay, String serial) { + /* "" means null (a default annotation String value cannot be null) */ + if (serial == null || serial.isEmpty()) { + if (delay > 0) { + /* no serial, but a delay: schedule the task */ + if (!(executor instanceof ScheduledExecutorService)) { + throw new IllegalArgumentException("The executor set does not support scheduling"); + } + ((ScheduledExecutorService) executor).schedule(runnable, delay, TimeUnit.MILLISECONDS); + } else { + /* no serial, no delay: execute now */ + executor.execute(runnable); } - ((ScheduledExecutorService) executor).schedule(runnable, delay, TimeUnit.MILLISECONDS); } else { - /* execute now */ - executor.execute(runnable); + /* serial is defined, the delay is managed by Task */ + Task task = new Task(runnable, delay, serial); + + synchronized (serialRunning) { + if (serialRunning.contains(serial)) { + /* a task for this serial is already running, queue this one */ + List queue = serialQueues.get(serial); + if (queue == null) { + /* the queue does not exist yet */ + queue = new ArrayList(); + serialQueues.put(serial, queue); + } + /* queue the task for later execution */ + queue.add(task); + } else { + /* mark this serial as having a running task */ + serialRunning.add(serial); + /* execute the task (a wrapper for runnable) now */ + execute(task, delay); /* do not pass serial here */ + } + } } } /** * Execute a task. * + * Equivalent to {@link #execute(Runnable, int, String) execute(runnable, 0, + * null)}. + * * @param runnable * the task to execute */ public static void execute(Runnable runnable) { - execute(runnable, 0); + execute(runnable, 0, null); + } + + /** + * Execute a task after the given delay. + * + * Equivalent to {@link #execute(Runnable, int, String) execute(runnable, + * delay, null)}. + * + * @param runnable + * the task to execute + * @param delay + * the time from now to delay execution, in milliseconds + * @throws IllegalArgumentException + * if delay is strictly positive and the current + * executor does not support scheduling (if + * {@link #setExecutor(Executor)} has been called with such an + * executor) + */ + public static void execute(Runnable runnable, int delay) { + execute(runnable, delay, null); + } + + /** + * Execute a task after all tasks added with the same non-null + * serial (if any) have completed execution. + * + * Equivalent to {@link #execute(Runnable, int, String) execute(runnable, 0, + * serial)}. + * + * @param runnable + * the task to execute + * @param serial + * the serial queue to use (null or "" + * for no serial execution) + */ + public static void execute(Runnable runnable, String serial) { + execute(runnable, 0, serial); } /** @@ -72,4 +160,54 @@ public static void setExecutor(Executor executor) { BackgroundExecutor.executor = executor; } + private static class Task implements Runnable { + + Runnable runnable; + long targetTime; /* in milliseconds since epoch */ + String serial; + + Task(Runnable runnable, int delay, String serial) { + this.runnable = runnable; + if (delay > 0) { + targetTime = System.currentTimeMillis() + delay; + } + this.serial = serial; + } + + @Override + public void run() { + try { + runnable.run(); + } finally { + /* handle next tasks */ + postExecute(); + } + } + + private void postExecute() { + synchronized (serialRunning) { + List queue = serialQueues.get(serial); + if (queue == null) { + /* no task is queue for this serial, mark it as not running */ + serialRunning.remove(serial); + } else { + /* queue is not empty, retrieve the oldest queued task */ + Task nextTask = queue.remove(0); + + if (queue.isEmpty()) { + /* no more tasks in the queue */ + serialQueues.remove(serial); + } + + /* compute the remaining delay */ + int delay = Math.max(0, (int) (nextTask.targetTime - System.currentTimeMillis())); + + /* execute the next task */ + execute(nextTask, delay); /* do not pass serial here */ + } + } + } + + } + } diff --git a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java index 130e47f328..9d03283085 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java @@ -55,12 +55,13 @@ public void process(Element element, JCodeModel codeModel, EBeanHolder holder) t { // Execute Runnable Background annotation = element.getAnnotation(Background.class); - long delay = annotation.delay(); + int delay = annotation.delay(); + String serial = annotation.serial(); JClass backgroundExecutorClass = holder.refClass(BackgroundExecutor.class); JInvocation executeCall; - executeCall = backgroundExecutorClass.staticInvoke("execute").arg(_new(anonymousRunnableClass)).arg(lit(delay)); + executeCall = backgroundExecutorClass.staticInvoke("execute").arg(_new(anonymousRunnableClass)).arg(lit(delay)).arg(lit(serial)); delegatingMethod.body().add(executeCall); From 2ded3aa02ed55a2c3153e99f3e3b4e0251f6ef34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Thu, 18 Apr 2013 14:53:33 +0200 Subject: [PATCH 3/5] Test for @Background(serial="some_id") Test if actions are sequential as expected. --- .../test15/ThreadActivityTest.java | 45 +++++++++++++++++++ .../test15/ThreadActivity.java | 14 ++++++ 2 files changed, 59 insertions(+) diff --git a/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java b/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java index 23c53fe4ce..c9ccb647cf 100644 --- a/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java +++ b/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java @@ -19,8 +19,14 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.Semaphore; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -51,4 +57,43 @@ public void backgroundDelegatesToExecutor() { verify(executor).execute(Mockito.any()); } + /** + * Start several requests which add an item to a list in background, with + * "@Background" serial attribute enabled, so the request must be executed + * sequentially. + * + * Once all tasks have completed execution, check if the items in the list + * are ordered. + */ + @Test + public void serializedBackgroundTasks() { + /* number of items to add to the list */ + final int NB_ADD = 10; + + /* set an executor with 4 threads */ + BackgroundExecutor.setExecutor(Executors.newFixedThreadPool(4)); + + /* the calls are serialized, but not necessarily on the same thread, so we + * need to synchronize to avoid cache effects */ + List list = Collections.synchronizedList(new ArrayList()); + + /* sem.acquire() will be unlocked exactly after NB_ADD releases */ + Semaphore sem = new Semaphore(1 - NB_ADD); + + /* execute NB_ADD requests to add an item to the list */ + for (int i = 0; i < NB_ADD; i++) { + activity.addSerializedBackgroundMethod(list, i, sem); + } + + try { + /* wait for all tasks to be completed */ + sem.acquire(); + + /* check if list items are in the right order */ + for (int i = 0; i < NB_ADD; i++) { + Assert.assertEquals("Items must be in order", i, (int) list.get(i)); + } + } catch (InterruptedException e) {} + } + } diff --git a/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java b/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java index 83410dd0de..0ac80c64a1 100644 --- a/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java +++ b/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java @@ -17,7 +17,9 @@ import java.util.List; import java.util.Map; +import java.util.Random; import java.util.Set; +import java.util.concurrent.Semaphore; import org.androidannotations.annotations.Background; import org.androidannotations.annotations.EActivity; @@ -46,6 +48,18 @@ void emptyDelayedBackgroundMethod() { } + @Background(serial="test") + void addSerializedBackgroundMethod(List list, int i, Semaphore sem) { + /* wait a random delay to increase the probability of wrong order if buggy */ + try { + /* wait between 0 and 20 milliseconds */ + Thread.sleep(new Random().nextInt(20)); + } catch (InterruptedException e) {} + + list.add(i); + sem.release(); + } + @UiThread void objectUiMethod(Object param) { From d39e77d6ede04d930bd5ba90f1f25833e6d88a7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Fri, 19 Apr 2013 09:32:17 +0200 Subject: [PATCH 4/5] Comment typo fix --- .../java/org/androidannotations/test15/ThreadActivityTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java b/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java index c9ccb647cf..f2330d0608 100644 --- a/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java +++ b/AndroidAnnotations/functional-test-1-5-tests/src/test/java/org/androidannotations/test15/ThreadActivityTest.java @@ -59,7 +59,7 @@ public void backgroundDelegatesToExecutor() { /** * Start several requests which add an item to a list in background, with - * "@Background" serial attribute enabled, so the request must be executed + * "@Background" serial attribute enabled, so the requests must be executed * sequentially. * * Once all tasks have completed execution, check if the items in the list From 2045e41b65bb741a00d27de3b592240ee6f696d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Fri, 19 Apr 2013 09:32:36 +0200 Subject: [PATCH 5/5] Use SystemClock.sleep() Avoid try/catch InterruptedException of Thread.sleep(). --- .../org/androidannotations/test15/ThreadActivity.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java b/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java index 0ac80c64a1..41d2395d86 100644 --- a/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java +++ b/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/ThreadActivity.java @@ -29,6 +29,7 @@ import org.androidannotations.test15.instancestate.MySerializableBean; import android.app.Activity; +import android.os.SystemClock; @EActivity public class ThreadActivity extends Activity { @@ -50,12 +51,9 @@ void emptyDelayedBackgroundMethod() { @Background(serial="test") void addSerializedBackgroundMethod(List list, int i, Semaphore sem) { - /* wait a random delay to increase the probability of wrong order if buggy */ - try { - /* wait between 0 and 20 milliseconds */ - Thread.sleep(new Random().nextInt(20)); - } catch (InterruptedException e) {} - + /* wait a random delay (between 0 and 20 milliseconds) to increase the + * probability of wrong order if buggy */ + SystemClock.sleep(new Random().nextInt(20)); list.add(i); sem.release(); }