Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Add delay in @Background #525

Merged
mathieuboniface merged 4 commits into from
Apr 13, 2013
Merged

Add delay in @Background #525

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
044017c
Add delay in @Background
Mar 4, 2013
31adbd8
Use ScheduledExecutorService instead of Handler
Mar 6, 2013
8fc6e05
Remove unused method
Mar 11, 2013
98c361a
Change corePoolSize from 1 to 2 * Runtime.availableProcessors()
Mar 13, 2013
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
Prev Previous commit
Next Next commit
Use ScheduledExecutorService instead of Handler
  • Loading branch information
Yoann Delouis
Yoann Delouis committed Mar 6, 2013
commit 31adbd84d836a8a0c74da08c0565781a2465c690
12 changes: 11 additions & 1 deletion ...tions/androidannotations/src/main/java/org/androidannotations/api/BackgroundExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,27 @@

import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

public class BackgroundExecutor {

private static Executor executor = Executors.newCachedThreadPool();
private static ScheduledExecutorService scheduledExecutor = Executors.newScheduledThreadPool(1);

@pyricau pyricau Mar 9, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The core pool size set to 1, which AFAIK means the maximum number of threads on this executor will always 1. As a consequence, you'll never get two delayed tasks to run concurrently. Is that on purpose ? @Background has always had a parallel behavior, if we want serial behavior which should make a clear distinction.

@yDelouis yDelouis Mar 11, 2013

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't know which value to set here. Maybe a big one (ex: Integer.MAX_VALUE) or maybe we should create a ScheduledExecutorService each time we call executeDelayed ? Do you know the effect of a value of 0 for the core pool size ? It's not said in the java doc and it doesn't seem to rise an exception.

@ThomasFondrillon ThomasFondrillon Mar 12, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It may be a good thing to let the user defines how many threads he wants because this is typically a parameter that changes according to the needs.

Maybe with another optional parameter in the @Background annotation. If the user doesn't fill this parameter, assign a default value to 5 for the corePoolSize.

What do you think about this idea?

@mathieuboniface mathieuboniface Mar 12, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think it's a good idea as it means each @Background could be responsible of the creation of a thread pool. That could cause a performance issue, and it could be hard to understand from a developer point of view.

I would prefer to define a value. Something like that :

What do you think ?

@yDelouis yDelouis Mar 12, 2013

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think Mathieu is right. The more so as a developper will be able to change the ScheduledExecutorService using the static method of BackgroundExecutor. Regarding the value to set, I have no idea.

@DayS DayS Mar 12, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍


public static void execute(Runnable runnable) {
executor.execute(runnable);
}

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;
}
}
16 changes: 5 additions & 11 deletions ...droidannotations/src/main/java/org/androidannotations/processing/BackgroundProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import static com.sun.codemodel.JExpr._new;
import static com.sun.codemodel.JExpr.lit;

import java.lang.annotation.Annotation;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;

Expand All @@ -30,10 +29,8 @@
import com.sun.codemodel.JClassAlreadyExistsException;
import com.sun.codemodel.JCodeModel;
import com.sun.codemodel.JDefinedClass;
import com.sun.codemodel.JExpr;
import com.sun.codemodel.JInvocation;
import com.sun.codemodel.JMethod;
import com.sun.codemodel.JMod;

public class BackgroundProcessor implements DecoratingElementProcessor {

Expand Down Expand Up @@ -61,19 +58,16 @@ public void process(Element element, JCodeModel codeModel, EBeanHolder holder) t
long delay = annotation.delay();

JClass backgroundExecutorClass = holder.refClass(BackgroundExecutor.class);
JInvocation executeCall = backgroundExecutorClass.staticInvoke("execute").arg(_new(anonymousRunnableClass));
JInvocation executeCall;

if (delay == 0) {
delegatingMethod.body().add(executeCall);
executeCall = backgroundExecutorClass.staticInvoke("execute").arg(_new(anonymousRunnableClass));
} else {
JDefinedClass anonymousExecutorRunnableClass = helper.createAnonymousRunnableClass(holder, executeCall);
if (holder.handler == null) {
JClass handlerClass = holder.classes().HANDLER;
holder.handler = holder.generatedClass.field(JMod.PRIVATE, handlerClass, "handler_", _new(handlerClass));
}
delegatingMethod.body().invoke(holder.handler, "postDelayed").arg(_new(anonymousExecutorRunnableClass)).arg(lit(delay));
executeCall = backgroundExecutorClass.staticInvoke("executeDelayed").arg(_new(anonymousRunnableClass)).arg(lit(delay));
}

delegatingMethod.body().add(executeCall);

}

}
Expand Down