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 01/17] 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 02/17] 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 03/17] 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 04/17] 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 05/17] 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(); } From 5254f56f77a852c0da28a44f8b01cb7892f53e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Sun, 21 Apr 2013 14:51:26 +0200 Subject: [PATCH 06/17] Better task manager implementation The previous implementation used a set for serial running and a map of lists of tasks for managing the serial queues. In practice, there will be very few parallel tasks, using maps and creating/destroying lists is complex and unefficient. A better approach is to use only a list of tasks (only the ones we need to keep, having a non-null serial) and run through it sequentially for retrieving tasks. Moreover, it is more general, and paves the way for adding a task cancellation feature. --- .../api/BackgroundExecutor.java | 214 +++++++++++------- 1 file changed, 126 insertions(+), 88 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 b5050519ed..dce4ffec70 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -16,11 +16,7 @@ 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; @@ -30,92 +26,86 @@ 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>(); + private static final List tasks = new ArrayList(); /** - * 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. + * Execute a runnable after the given delay. * * @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, 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); + private static void directExecute(Runnable runnable, int delay) { + 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 { - /* 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 */ - } - } + /* no serial, no delay: execute now */ + executor.execute(runnable); } } /** - * Execute a task. + * Execute a task after (at least) its delay and 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, - * null)}. + * @param task + * the task to execute + * @throws IllegalArgumentException + * if task.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 synchronized void execute(Task task) { + if (task.serial == null || !hasSerialRunning(task.serial)) { + task.executionAsked = true; + directExecute(task, task.delay); + } + if (task.serial != null) { + /* keep task */ + tasks.add(task); + } + } + + /** + * Execute a task. * * @param runnable * the task to execute + * @param delay + * the time from now to delay execution, in milliseconds + * @param serial + * the serial queue (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) { - execute(runnable, 0, null); + public static void execute(final Runnable runnable, int delay, String serial) { + execute(new Task(delay, serial) { + @Override + public void execute() { + runnable.run(); + } + }); } /** * 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 @@ -127,7 +117,17 @@ public static void execute(Runnable runnable) { * executor) */ public static void execute(Runnable runnable, int delay) { - execute(runnable, delay, null); + directExecute(runnable, delay); + } + + /** + * Execute a task. + * + * @param runnable + * the task to execute + */ + public static void execute(Runnable runnable) { + directExecute(runnable, 0); } /** @@ -160,50 +160,88 @@ public static void setExecutor(Executor executor) { BackgroundExecutor.executor = executor; } - private static class Task implements Runnable { + /** + * Indicates whether a task with the specified serial has been + * submitted to the executor. + * + * @param serial + * the serial queue + * @return true if such a task has been submitted, + * false otherwise + */ + private static boolean hasSerialRunning(String serial) { + for (Task task : tasks) { + if (task.executionAsked && serial.equals(task.serial)) { + return true; + } + } + return false; + } + + /** + * Retrieve and remove the first task having the specified + * serial (if any). + * + * @param serial + * the serial queue + * @return task if found, null otherwise + */ + private static Task take(String serial) { + int len = tasks.size(); + for (int i = 0; i < len; i++) { + if (serial.equals(tasks.get(i).serial)) { + return tasks.remove(i); + } + } + return null; + } + + public static abstract class Task implements Runnable { - Runnable runnable; - long targetTime; /* in milliseconds since epoch */ - String serial; + private int delay; + private long targetTime; /* in milliseconds since epoch */ + private String serial; + private boolean executionAsked; - Task(Runnable runnable, int delay, String serial) { - this.runnable = runnable; + public Task(int delay, String serial) { if (delay > 0) { + this.delay = delay; targetTime = System.currentTimeMillis() + delay; } - this.serial = serial; + if (!"".equals(serial)) { + this.serial = serial; + } } @Override public void run() { try { - runnable.run(); + execute(); } finally { /* handle next tasks */ postExecute(); } } + public abstract void execute(); + 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); + if (serial == null) { + /* nothing to do */ + return; + } + synchronized (BackgroundExecutor.class) { + /* execution complete */ + tasks.remove(this); + + Task next = take(serial); + if (next != null) { + if (next.delay != 0) { + /* compute remaining delay */ + next.delay = Math.max(0, (int) (targetTime - System.currentTimeMillis())); } - - /* 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 */ + /* a task having the same serial was queued, execute it */ + BackgroundExecutor.execute(next); } } } From 1ab7373fecc389a165580d9b74b6a94cb8bc8faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Mon, 22 Apr 2013 09:56:00 +0200 Subject: [PATCH 07/17] Factorize surround with try/catch generation Extract the surround-with-try-catch part of the anonymous Runnable generation, in order to reuse it for generating other anonymous classes. --- .../helper/APTCodeModelHelper.java | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/APTCodeModelHelper.java b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/APTCodeModelHelper.java index 1648c76447..4153fec49a 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/APTCodeModelHelper.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/APTCodeModelHelper.java @@ -228,6 +228,20 @@ public String getIdStringFromIdFieldRef(JFieldRef idRef) { throw new IllegalStateException("Unable to extract target name from JFieldRef"); } + public JTryBlock surroundWithTryCatch(EBeanHolder holder, JBlock block, JBlock content, String exceptionMessage) { + Classes classes = holder.classes(); + JTryBlock tryBlock = block._try(); + tryBlock.body().add(content); + JCatchBlock catchBlock = tryBlock._catch(classes.RUNTIME_EXCEPTION); + JVar exceptionParam = catchBlock.param("e"); + JInvocation errorInvoke = classes.LOG.staticInvoke("e"); + errorInvoke.arg(holder.generatedClass.name()); + errorInvoke.arg(exceptionMessage); + errorInvoke.arg(exceptionParam); + catchBlock.body().add(errorInvoke); + return tryBlock; + } + public JDefinedClass createDelegatingAnonymousRunnableClass(EBeanHolder holder, JMethod delegatedMethod) { JCodeModel codeModel = holder.codeModel(); @@ -242,20 +256,9 @@ public JDefinedClass createDelegatingAnonymousRunnableClass(EBeanHolder holder, runMethod.annotate(Override.class); JBlock runMethodBody = runMethod.body(); - JTryBlock runTry = runMethodBody._try(); - runTry.body().add(previousMethodBody); - - JCatchBlock runCatch = runTry._catch(classes.RUNTIME_EXCEPTION); - JVar exceptionParam = runCatch.param("e"); - - JInvocation errorInvoke = classes.LOG.staticInvoke("e"); - - errorInvoke.arg(holder.generatedClass.name()); - errorInvoke.arg("A runtime exception was thrown while executing code in a runnable"); - errorInvoke.arg(exceptionParam); + surroundWithTryCatch(holder, runMethodBody, previousMethodBody, "A runtime exception was thrown while executing code in a runnable"); - runCatch.body().add(errorInvoke); return anonymousRunnableClass; } From ec9fea33efc3c96d7cc809e3de4c4dc3b10bac9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Mon, 22 Apr 2013 09:56:16 +0200 Subject: [PATCH 08/17] Directly generate Task instantiation Instead of create a new Runnable instance which will be wrapped by a Task instance, directly create a Task (which is a Runnable). --- .../processing/BackgroundProcessor.java | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) 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 9d03283085..bf3c03892b 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java @@ -23,14 +23,17 @@ import org.androidannotations.annotations.Background; import org.androidannotations.api.BackgroundExecutor; +import org.androidannotations.api.BackgroundExecutor.Task; import org.androidannotations.helper.APTCodeModelHelper; +import com.sun.codemodel.JBlock; import com.sun.codemodel.JClass; import com.sun.codemodel.JClassAlreadyExistsException; import com.sun.codemodel.JCodeModel; import com.sun.codemodel.JDefinedClass; import com.sun.codemodel.JInvocation; import com.sun.codemodel.JMethod; +import com.sun.codemodel.JMod; public class BackgroundProcessor implements DecoratingElementProcessor { @@ -50,22 +53,24 @@ public void process(Element element, JCodeModel codeModel, EBeanHolder holder) t JMethod delegatingMethod = helper.overrideAnnotatedMethod(executableElement, holder); - JDefinedClass anonymousRunnableClass = helper.createDelegatingAnonymousRunnableClass(holder, delegatingMethod); + JBlock previousMethodBody = helper.removeBody(delegatingMethod); - { - // Execute Runnable - Background annotation = element.getAnnotation(Background.class); - int delay = annotation.delay(); - String serial = annotation.serial(); + JDefinedClass anonymousTaskClass = codeModel.anonymousClass(Task.class); - JClass backgroundExecutorClass = holder.refClass(BackgroundExecutor.class); - JInvocation executeCall; + JMethod executeMethod = anonymousTaskClass.method(JMod.PUBLIC, codeModel.VOID, "execute"); + executeMethod.annotate(Override.class); - executeCall = backgroundExecutorClass.staticInvoke("execute").arg(_new(anonymousRunnableClass)).arg(lit(delay)).arg(lit(serial)); + JBlock runMethodBody = executeMethod.body(); + helper.surroundWithTryCatch(holder, runMethodBody, previousMethodBody, "A runtime exception was thrown while executing code in a background task"); - delegatingMethod.body().add(executeCall); + Background annotation = element.getAnnotation(Background.class); + String id = annotation.id(); + int delay = annotation.delay(); - } + JClass backgroundExecutorClass = holder.refClass(BackgroundExecutor.class); + JInvocation newTask = _new(anonymousTaskClass).arg(lit(id)).arg(lit(delay)); + JInvocation executeCall = backgroundExecutorClass.staticInvoke("execute").arg(newTask); + delegatingMethod.body().add(executeCall); } } From 99ca871cd85b1b7438dd69e8fb0a59385d480307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Sun, 21 Apr 2013 15:18:55 +0200 Subject: [PATCH 09/17] Cancellable background tasks Adds an optional id attribute to @Background: @Background(id="some_id") The user is able to cancel all background tasks having a specified id: BackgroundExecutor.cancelAll("some_id"); --- .../annotations/Background.java | 2 + .../api/BackgroundExecutor.java | 90 ++++++++++++++----- .../processing/BackgroundProcessor.java | 3 +- 3 files changed, 73 insertions(+), 22 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 7007d3c7fa..ebec74aaba 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,6 +28,8 @@ @Retention(RetentionPolicy.CLASS) @Target(ElementType.METHOD) public @interface Background { + String id() default ""; /* used for task cancellation */ + 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 dce4ffec70..2b825d2448 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -18,7 +18,9 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -40,18 +42,26 @@ public class BackgroundExecutor { * executor does not support scheduling (if * {@link #setExecutor(Executor)} has been called with such an * executor) + * @return Future associated to the running task */ - private static void directExecute(Runnable runnable, int delay) { + private static Future directExecute(Runnable runnable, int delay) { + Future future = null; 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); + future = ((ScheduledExecutorService) executor).schedule(runnable, delay, TimeUnit.MILLISECONDS); } else { /* no serial, no delay: execute now */ - executor.execute(runnable); + if (executor instanceof ExecutorService) { + future = ((ExecutorService) executor).submit(runnable); + } else { + /* non-cancellable task */ + executor.execute(runnable); + } } + return future; } /** @@ -68,12 +78,17 @@ private static void directExecute(Runnable runnable, int delay) { * executor) */ public static synchronized void execute(Task task) { + Future future = null; if (task.serial == null || !hasSerialRunning(task.serial)) { task.executionAsked = true; - directExecute(task, task.delay); + future = directExecute(task, task.delay); + if (task.id != null && future == null) { + throw new IllegalArgumentException("The executor set does not support task cancellation"); + } } - if (task.serial != null) { + if (task.id != null || task.serial != null) { /* keep task */ + task.future = future; tasks.add(task); } } @@ -83,6 +98,8 @@ public static synchronized void execute(Task task) { * * @param runnable * the task to execute + * @param id + * identifier used for task cancellation * @param delay * the time from now to delay execution, in milliseconds * @param serial @@ -94,8 +111,8 @@ public static synchronized void execute(Task task) { * {@link #setExecutor(Executor)} has been called with such an * executor) */ - public static void execute(final Runnable runnable, int delay, String serial) { - execute(new Task(delay, serial) { + public static void execute(final Runnable runnable, String id, int delay, String serial) { + execute(new Task(id, delay, serial) { @Override public void execute() { runnable.run(); @@ -134,24 +151,28 @@ public static void execute(Runnable runnable) { * 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)}. + * Equivalent to {@link #execute(Runnable, String, int, String) + * execute(runnable, id, 0, serial)}. * * @param runnable * the task to execute + * @param id + * identifier used for task cancellation * @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); + public static void execute(Runnable runnable, String id, String serial) { + execute(runnable, id, 0, serial); } /** * 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. + * then executing a task after a delay will not be supported anymore. If it + * is not even a {@link ExecutorService} then tasks will not be cancellable + * anymore. * * @param executor * the new executor @@ -160,6 +181,26 @@ public static void setExecutor(Executor executor) { BackgroundExecutor.executor = executor; } + /** + * Cancel all tasks having the specified id. + * + * @param id + * the cancellation identifier + * @param mayInterruptIfRunning + * true<.code> if the thread executing this task should be interrupted; otherwise, in-progress tasks are allowed to complete + */ + public static synchronized void cancelAll(String id, boolean mayInterruptIfRunning) { + for (int i = tasks.size() - 1; i >= 0; i--) { + Task task = tasks.get(i); + if (id.equals(task.id)) { + tasks.remove(i); + if (task.future != null) { + task.future.cancel(mayInterruptIfRunning); + } + } + } + } + /** * Indicates whether a task with the specified serial has been * submitted to the executor. @@ -198,12 +239,17 @@ private static Task take(String serial) { public static abstract class Task implements Runnable { + private String id; private int delay; private long targetTime; /* in milliseconds since epoch */ private String serial; private boolean executionAsked; + private Future future; - public Task(int delay, String serial) { + public Task(String id, int delay, String serial) { + if (!"".equals(id)) { + this.id = id; + } if (delay > 0) { this.delay = delay; targetTime = System.currentTimeMillis() + delay; @@ -226,7 +272,7 @@ public void run() { public abstract void execute(); private void postExecute() { - if (serial == null) { + if (id == null && serial == null) { /* nothing to do */ return; } @@ -234,14 +280,16 @@ private void postExecute() { /* execution complete */ tasks.remove(this); - Task next = take(serial); - if (next != null) { - if (next.delay != 0) { - /* compute remaining delay */ - next.delay = Math.max(0, (int) (targetTime - System.currentTimeMillis())); + if (serial != null) { + Task next = take(serial); + if (next != null) { + if (next.delay != 0) { + /* compute remaining delay */ + next.delay = Math.max(0, (int) (targetTime - System.currentTimeMillis())); + } + /* a task having the same serial was queued, execute it */ + BackgroundExecutor.execute(next); } - /* a task having the same serial was queued, execute it */ - BackgroundExecutor.execute(next); } } } 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 bf3c03892b..896839e428 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java @@ -66,9 +66,10 @@ public void process(Element element, JCodeModel codeModel, EBeanHolder holder) t Background annotation = element.getAnnotation(Background.class); String id = annotation.id(); int delay = annotation.delay(); + String serial = annotation.serial(); JClass backgroundExecutorClass = holder.refClass(BackgroundExecutor.class); - JInvocation newTask = _new(anonymousTaskClass).arg(lit(id)).arg(lit(delay)); + JInvocation newTask = _new(anonymousTaskClass).arg(lit(id)).arg(lit(delay)).arg(lit(serial)); JInvocation executeCall = backgroundExecutorClass.staticInvoke("execute").arg(newTask); delegatingMethod.body().add(executeCall); From 0ec12b0db8a11a1cf834ed9cb607dd6a4cf8dd61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Mon, 22 Apr 2013 13:46:18 +0200 Subject: [PATCH 10/17] Tests for @Background attributes Tests the serialization and cancellation of background tasks. --- .../test15/ThreadActivityTest.java | 193 ++++++++++++++++-- .../test15/ThreadActivity.java | 41 +++- 2 files changed, 212 insertions(+), 22 deletions(-) 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 f2330d0608..5b379d3755 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 @@ -22,21 +22,24 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Random; import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import org.androidannotations.api.BackgroundExecutor; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; -import org.androidannotations.api.BackgroundExecutor; - @RunWith(AndroidAnnotationsTestRunner.class) public class ThreadActivityTest { + private static final int MAX_WAITING_TIME = 3000; /* milliseconds */ + private ThreadActivity_ activity; @Before @@ -58,12 +61,66 @@ public void backgroundDelegatesToExecutor() { } /** - * Start several requests which add an item to a list in background, with - * "@Background" serial attribute enabled, so the requests must be executed - * sequentially. + * Verify that non-serialized background tasks are not serialized (ensure that + * serial feature does not force all background tasks to be serialized). + * + * Start several requests which add an item to a list in background, without "@Background" + * serial attribute enabled. * - * Once all tasks have completed execution, check if the items in the list - * are ordered. + * Once all tasks have completed execution, verify that the items in the list are not ordered + * (with very little false-negative probability). + */ + @Test + public void parallelBackgroundTasks() { + /* number of items to add to the list */ + final int NB_ADD = 20; + + /* set an executor with 4 threads */ + BackgroundExecutor.setExecutor(Executors.newFixedThreadPool(4)); + + List list = Collections.synchronizedList(new ArrayList()); + + /* sem.acquire() will be unlocked exactly after NB_ADD releases */ + Semaphore sem = new Semaphore(1 - NB_ADD); + + Random random = new Random(); + + /* execute NB_ADD requests to add an item to the list */ + for (int i = 0; i < NB_ADD; i++) { + /* + * wait a random delay (between 0 and 20 milliseconds) to increase the probability of + * wrong order + */ + int delay = random.nextInt(20); + activity.addBackground(list, i, delay, sem); + } + + try { + /* wait for all tasks to be completed */ + boolean acquired = sem.tryAcquire(MAX_WAITING_TIME, TimeUnit.MILLISECONDS); + Assert.assertTrue("Requested tasks should have completed execution", acquired); + + /* + * verify that list items are in the wrong order (the probability it is in the right is + * 1/(NB_ADD!), which is nearly 0) + */ + boolean rightOrder = true; + for (int i = 0; i < NB_ADD && rightOrder; i++) { + rightOrder &= i == list.get(i); + } + Assert.assertFalse("Items should not be in order", rightOrder); + } catch (InterruptedException e) { + Assert.assertFalse("Testing thread should never be interrupted", true); + } + } + + /** + * Verify that serialized background tasks are correctly serialized. + * + * Start several requests which add an item to a list in background, with "@Background" serial + * attribute enabled, so the requests must be executed sequentially. + * + * Once all tasks have completed execution, verify that the items in the list are ordered. */ @Test public void serializedBackgroundTasks() { @@ -73,27 +130,137 @@ public void serializedBackgroundTasks() { /* 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 */ + /* + * 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); + Random random = new Random(); + /* execute NB_ADD requests to add an item to the list */ for (int i = 0; i < NB_ADD; i++) { - activity.addSerializedBackgroundMethod(list, i, sem); + /* + * wait a random delay (between 0 and 20 milliseconds) to increase the probability of + * wrong order if buggy + */ + int delay = random.nextInt(20); + activity.addSerializedBackground(list, i, delay, sem); } try { /* wait for all tasks to be completed */ - sem.acquire(); + boolean acquired = sem.tryAcquire(MAX_WAITING_TIME, TimeUnit.MILLISECONDS); + Assert.assertTrue("Requested tasks should have completed execution", acquired); - /* 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) {} + } catch (InterruptedException e) { + Assert.assertFalse("Testing thread should never be interrupted", true); + } + } + + /** + * Verify that cancellable background tasks are correctly cancelled, and others are not. + * + * Start several requests which add an item to a list in background, half explicitly cancelled, + * half not cancelled. + * + * Once all tasks have completed execution, check if and only if the items from the uncancelled + * tasks are the list. + */ + @Test + public void cancellableBackgroundTasks() { + /* 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 2*NB_ADD requests to add an item to the list, half being cancelled */ + for (int i = 0; i < NB_ADD; i++) { + activity.addBackground(list, i, 0, sem); + activity.addCancellableBackground(list, NB_ADD + i, 4000); + } + + /* cancel all tasks with id "to_cancel" */ + BackgroundExecutor.cancelAll("to_cancel", true); + + /* cancelled tasks won't have time to add any item */ + + try { + /* wait for all non cancelled tasks to be completed */ + boolean acquired = sem.tryAcquire(MAX_WAITING_TIME, TimeUnit.MILLISECONDS); + Assert.assertTrue("Requested tasks should have completed execution", acquired); + + Assert.assertEquals("Only uncancelled tasks must have added items", list.size(), NB_ADD); + + for (int i = 0; i < NB_ADD; i++) { + Assert.assertTrue("Items must be only from uncancelled tasks", i < NB_ADD); + } + } catch (InterruptedException e) { + Assert.assertFalse("Testing thread should never be interrupted", true); + } + } + + @Test + public void cancellableSerializedBackgroundTasks() { + /* number of items to add to the list */ + final int NB_ADD = 5; + + /* 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 2*NB_ADD requests to add an item to the list, half being cancelled */ + for (int i = 0; i < NB_ADD; i++) { + activity.addSerializedBackground(list, i, 0, sem); + activity.addCancellableSerializedBackground(list, NB_ADD + i, 4000); + } + + /* cancel all tasks with id "to_cancel_serial" */ + BackgroundExecutor.cancelAll("to_cancel_serial", true); + + /* cancelled tasks won't have time to add any item */ + + try { + /* wait for all non cancelled tasks to be completed */ + boolean acquired = sem.tryAcquire(MAX_WAITING_TIME, TimeUnit.MILLISECONDS); + Assert.assertTrue("Requested tasks should have completed execution", acquired); + + /* cancel all tasks with id "to_cancel_2" */ + BackgroundExecutor.cancelAll("to_cancel_2", true); + + Assert.assertEquals("Only uncancelled tasks must have added items", list.size(), NB_ADD); + + for (int i = 0; i < NB_ADD; i++) { + Assert.assertTrue("Items must be only from uncancelled tasks", i < NB_ADD); + } + + } catch (InterruptedException e) { + Assert.assertFalse("Testing thread should never be interrupted", true); + } } } 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 41d2395d86..58f79a63c9 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 @@ -43,19 +43,42 @@ void emptyUiMethod() { void emptyBackgroundMethod() { } - + @Background(delay = 1000) void emptyDelayedBackgroundMethod() { - + + } + + private void add(List list, int i, int delay, Semaphore sem) { + try { + if (delay > 0) { + Thread.sleep(delay); + } + list.add(i); + if (sem != null) { + sem.release(); + } + } catch (InterruptedException e) {} + } + + @Background + void addBackground(List list, int i, int delay, Semaphore sem) { + add(list, i, delay, sem); + } + + @Background(serial = "test") + void addSerializedBackground(List list, int i, int delay, Semaphore sem) { + add(list, i, delay, sem); + } + + @Background(id = "to_cancel") + void addCancellableBackground(List list, int i, int interruptibleDelay) { + add(list, i, interruptibleDelay, null); } - @Background(serial="test") - void addSerializedBackgroundMethod(List list, int i, Semaphore sem) { - /* 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(); + @Background(id = "to_cancel_serial", serial = "test") + void addCancellableSerializedBackground(List list, int i, int delay) { + add(list, i, delay, null); } @UiThread From dbd7cafdefb215c02b3242203c8f4481855bbbb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Mon, 22 Apr 2013 19:35:38 +0200 Subject: [PATCH 11/17] Do not fail if cancellation is not supported If the executor is replaced by another not supporting cancellation (not extending ExecutorService), it failed the first time a task with a non-null id was requested. Now, it just warns during cancellation. --- .../org/androidannotations/api/BackgroundExecutor.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 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 2b825d2448..a72fa7c2a9 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -24,8 +24,12 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import android.util.Log; + public class BackgroundExecutor { + private static final String TAG = "BackgroundExecutor"; + private static Executor executor = Executors.newScheduledThreadPool(2 * Runtime.getRuntime().availableProcessors()); private static final List tasks = new ArrayList(); @@ -82,9 +86,6 @@ public static synchronized void execute(Task task) { if (task.serial == null || !hasSerialRunning(task.serial)) { task.executionAsked = true; future = directExecute(task, task.delay); - if (task.id != null && future == null) { - throw new IllegalArgumentException("The executor set does not support task cancellation"); - } } if (task.id != null || task.serial != null) { /* keep task */ @@ -196,6 +197,8 @@ public static synchronized void cancelAll(String id, boolean mayInterruptIfRunni tasks.remove(i); if (task.future != null) { task.future.cancel(mayInterruptIfRunning); + } else if (task.executionAsked) { + Log.w(TAG, "A task with id " + task.id + " cannot be cancelled (the executor set does not support it)"); } } } From aa2fe11fba6a8bb9986ea042afb6a39f67dfccf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Tue, 21 May 2013 23:14:40 +0200 Subject: [PATCH 12/17] Comment bugfixes --- .../java/org/androidannotations/api/BackgroundExecutor.java | 4 +++- .../org/androidannotations/test15/ThreadActivityTest.java | 2 +- 2 files changed, 4 insertions(+), 2 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 a72fa7c2a9..cc070e4ddb 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -188,7 +188,9 @@ public static void setExecutor(Executor executor) { * @param id * the cancellation identifier * @param mayInterruptIfRunning - * true<.code> if the thread executing this task should be interrupted; otherwise, in-progress tasks are allowed to complete + * true if the thread executing this task should be + * interrupted; otherwise, in-progress tasks are allowed to + * complete */ public static synchronized void cancelAll(String id, boolean mayInterruptIfRunning) { for (int i = tasks.size() - 1; i >= 0; i--) { 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 5b379d3755..00fda24974 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 @@ -171,7 +171,7 @@ public void serializedBackgroundTasks() { * half not cancelled. * * Once all tasks have completed execution, check if and only if the items from the uncancelled - * tasks are the list. + * tasks are in the list. */ @Test public void cancellableBackgroundTasks() { From fddcc1b68fe156a14fb80557776c84a29f456d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Wed, 22 May 2013 15:15:36 +0200 Subject: [PATCH 13/17] Cast into a local variable before dereferencing https://github.com/rom1v/androidannotations/commit/879f196e4fcfd647ad0b674827ca22ef51459953#commitcomment-3261065 --- .../java/org/androidannotations/api/BackgroundExecutor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 cc070e4ddb..d5c21ac178 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -55,11 +55,13 @@ private static Future directExecute(Runnable runnable, int delay) { if (!(executor instanceof ScheduledExecutorService)) { throw new IllegalArgumentException("The executor set does not support scheduling"); } - future = ((ScheduledExecutorService) executor).schedule(runnable, delay, TimeUnit.MILLISECONDS); + ScheduledExecutorService scheduledExecutorService = (ScheduledExecutorService) executor; + future = scheduledExecutorService.schedule(runnable, delay, TimeUnit.MILLISECONDS); } else { /* no serial, no delay: execute now */ if (executor instanceof ExecutorService) { - future = ((ExecutorService) executor).submit(runnable); + ExecutorService executorService = (ExecutorService) executor; + future = executorService.submit(runnable); } else { /* non-cancellable task */ executor.execute(runnable); From 33b3cf492e041b9927e663fea4ce8040af9c96c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Wed, 22 May 2013 15:24:59 +0200 Subject: [PATCH 14/17] Remove useless comment https://github.com/rom1v/androidannotations/commit/879f196e4fcfd647ad0b674827ca22ef51459953#commitcomment-3261070 --- .../main/java/org/androidannotations/api/BackgroundExecutor.java | 1 - 1 file changed, 1 deletion(-) 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 d5c21ac178..902bb9dc9a 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -58,7 +58,6 @@ private static Future directExecute(Runnable runnable, int delay) { ScheduledExecutorService scheduledExecutorService = (ScheduledExecutorService) executor; future = scheduledExecutorService.schedule(runnable, delay, TimeUnit.MILLISECONDS); } else { - /* no serial, no delay: execute now */ if (executor instanceof ExecutorService) { ExecutorService executorService = (ExecutorService) executor; future = executorService.submit(runnable); From 75b15167062a43bf83d1f9210d6f95cd1a091528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Wed, 22 May 2013 15:26:06 +0200 Subject: [PATCH 15/17] Refactor targetTime -> targetTimeMillis https://github.com/rom1v/androidannotations/commit/879f196e4fcfd647ad0b674827ca22ef51459953#commitcomment-3260958 --- .../java/org/androidannotations/api/BackgroundExecutor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 902bb9dc9a..559b86b2fe 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -247,7 +247,7 @@ public static abstract class Task implements Runnable { private String id; private int delay; - private long targetTime; /* in milliseconds since epoch */ + private long targetTimeMillis; /* since epoch */ private String serial; private boolean executionAsked; private Future future; @@ -258,7 +258,7 @@ public Task(String id, int delay, String serial) { } if (delay > 0) { this.delay = delay; - targetTime = System.currentTimeMillis() + delay; + targetTimeMillis = System.currentTimeMillis() + delay; } if (!"".equals(serial)) { this.serial = serial; @@ -291,7 +291,7 @@ private void postExecute() { if (next != null) { if (next.delay != 0) { /* compute remaining delay */ - next.delay = Math.max(0, (int) (targetTime - System.currentTimeMillis())); + next.delay = Math.max(0, (int) (targetTimeMillis - System.currentTimeMillis())); } /* a task having the same serial was queued, execute it */ BackgroundExecutor.execute(next); From f3a5718afda601f8abd28a9821afb072721ab254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Wed, 22 May 2013 15:35:22 +0200 Subject: [PATCH 16/17] Refactor delay -> remainingDelay https://github.com/rom1v/androidannotations/commit/879f196e4fcfd647ad0b674827ca22ef51459953#commitcomment-3261105 --- .../androidannotations/api/BackgroundExecutor.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 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 559b86b2fe..f757ffc656 100644 --- a/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -86,7 +86,7 @@ public static synchronized void execute(Task task) { Future future = null; if (task.serial == null || !hasSerialRunning(task.serial)) { task.executionAsked = true; - future = directExecute(task, task.delay); + future = directExecute(task, task.remainingDelay); } if (task.id != null || task.serial != null) { /* keep task */ @@ -246,7 +246,7 @@ private static Task take(String serial) { public static abstract class Task implements Runnable { private String id; - private int delay; + private int remainingDelay; private long targetTimeMillis; /* since epoch */ private String serial; private boolean executionAsked; @@ -257,7 +257,7 @@ public Task(String id, int delay, String serial) { this.id = id; } if (delay > 0) { - this.delay = delay; + remainingDelay = delay; targetTimeMillis = System.currentTimeMillis() + delay; } if (!"".equals(serial)) { @@ -289,9 +289,9 @@ private void postExecute() { if (serial != null) { Task next = take(serial); if (next != null) { - if (next.delay != 0) { - /* compute remaining delay */ - next.delay = Math.max(0, (int) (targetTimeMillis - System.currentTimeMillis())); + if (next.remainingDelay != 0) { + /* the delay may not have elapsed yet */ + next.remainingDelay = Math.max(0, (int) (targetTimeMillis - System.currentTimeMillis())); } /* a task having the same serial was queued, execute it */ BackgroundExecutor.execute(next); From 4161c6233a44f81a51e295d494413df7c007bf23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Vimont=20=28=C2=AEom=29?= Date: Wed, 22 May 2013 15:50:03 +0200 Subject: [PATCH 17/17] Javadoc for @Background annotation https://github.com/excilys/androidannotations/pull/564/files#r4329001 --- .../annotations/Background.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 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 ebec74aaba..1302ab7023 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,9 +28,26 @@ @Retention(RetentionPolicy.CLASS) @Target(ElementType.METHOD) public @interface Background { - String id() default ""; /* used for task cancellation */ + /** + * Identifier for task cancellation. + * + * To cancel all tasks having a specified background id: + * + * 
+	 * boolean mayInterruptIfRunning = true;
+	 * BackgroundExecutor.cancelAll("my_background_id", mayInterruptIfRunning);
+	 *
+ **/ + String id() default ""; - int delay() default 0; /* in milliseconds */ + /** Minimum delay, in milliseconds, before the background task is executed. */ + int delay() default 0; + /** + * Serial execution group. + * + * All background tasks having the same serial will be executed + * sequentially. + **/ String serial() default ""; }