From 37c624af54a0228c409fdcbef1772a6f7c4facef Mon Sep 17 00:00:00 2001 From: Ruby Hartono <58564005+rh-id@users.noreply.github.com> Date: Fri, 19 Nov 2021 15:21:54 +0700 Subject: [PATCH] [Performance] Optimize thread pool usage across all provider instances This prevents ThreadPool threads to grow too large when using registerAsync --- .../m/co/rh/id/aprovider/DefaultProvider.java | 18 ++++++------- .../aprovider/LazyFutureProviderRegister.java | 25 +++++++++---------- .../co/rh/id/aprovider/ProviderUnitTest.java | 7 +++++- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/provider/src/main/java/m/co/rh/id/aprovider/DefaultProvider.java b/provider/src/main/java/m/co/rh/id/aprovider/DefaultProvider.java index adcb19f..2c1b39a 100644 --- a/provider/src/main/java/m/co/rh/id/aprovider/DefaultProvider.java +++ b/provider/src/main/java/m/co/rh/id/aprovider/DefaultProvider.java @@ -10,8 +10,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -25,9 +24,8 @@ class DefaultProvider implements Provider, ProviderRegistry { private static synchronized ThreadPoolExecutor initThreadPool() { if (sThreadPoolExecutor == null) { - sThreadPoolExecutor = new ThreadPoolExecutor(Runtime.getRuntime().availableProcessors(), - Integer.MAX_VALUE, - 30, TimeUnit.SECONDS, new SynchronousQueue<>()); + sThreadPoolExecutor = new ThreadPoolExecutor(1, 1, 10, + TimeUnit.SECONDS, new LinkedBlockingQueue<>()); sThreadPoolExecutor.allowCoreThreadTimeOut(true); sThreadPoolExecutor.prestartAllCoreThreads(); } @@ -39,21 +37,21 @@ private static synchronized ThreadPoolExecutor initThreadPool() { private List mModuleList; private List mAsyncRegisterList; private Handler mHandler; - private ExecutorService mExecutorService; + private ThreadPoolExecutor mThreadPoolExecutor; private boolean mIsDisposed; DefaultProvider(Context context, ProviderModule rootModule) { this(context, rootModule, new Handler(Looper.getMainLooper()), initThreadPool()); } - DefaultProvider(Context context, ProviderModule rootModule, Handler handler, ExecutorService executorService) { + DefaultProvider(Context context, ProviderModule rootModule, Handler handler, ThreadPoolExecutor threadPoolExecutor) { mContext = context; mObjectMap = new ConcurrentHashMap<>(); mObjectMap.put(ProviderRegistry.class, this); mModuleList = Collections.synchronizedList(new ArrayList<>()); mAsyncRegisterList = Collections.synchronizedList(new ArrayList<>()); mHandler = handler; - mExecutorService = executorService; + mThreadPoolExecutor = threadPoolExecutor; registerModule(rootModule); // after all things are registered, trigger load for all futures loadAllAsyncRegisters(); @@ -123,7 +121,7 @@ private I exactGet(Class clazz) { @Override public void getAsyncAndDo(Class clazz, ProviderAction actionOnMainThread) { - mExecutorService.execute(() -> { + mThreadPoolExecutor.execute(() -> { try { I value = get(clazz); mHandler.post(() -> actionOnMainThread.onSuccess(value)); @@ -177,7 +175,7 @@ public void registerLazy(Class clazz, ProviderValue providerValue) { @Override public void registerAsync(Class clazz, ProviderValue providerValue) { - LazyFutureProviderRegister providerRegister = new LazyFutureProviderRegister(clazz, providerValue, mExecutorService); + LazyFutureProviderRegister providerRegister = new LazyFutureProviderRegister(clazz, providerValue, mThreadPoolExecutor); register(providerRegister); mAsyncRegisterList.add(providerRegister); } diff --git a/provider/src/main/java/m/co/rh/id/aprovider/LazyFutureProviderRegister.java b/provider/src/main/java/m/co/rh/id/aprovider/LazyFutureProviderRegister.java index 9f9c5af..a607990 100644 --- a/provider/src/main/java/m/co/rh/id/aprovider/LazyFutureProviderRegister.java +++ b/provider/src/main/java/m/co/rh/id/aprovider/LazyFutureProviderRegister.java @@ -2,8 +2,8 @@ import android.util.Log; -import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +import java.util.concurrent.ThreadPoolExecutor; /** * Helper class to register lazy-loaded future singleton to the provider @@ -11,26 +11,25 @@ class LazyFutureProviderRegister extends ProviderRegister { private static final String TAG = "FutureProvider"; private Future mFutureValue; - private ExecutorService mExecutorService; + private ThreadPoolExecutor mThreadPoolExecutor; - public LazyFutureProviderRegister(Class type, ProviderValue providerValue, ExecutorService executorService) { + public LazyFutureProviderRegister(Class type, ProviderValue providerValue, ThreadPoolExecutor executorService) { super(type, providerValue); - mExecutorService = executorService; + mThreadPoolExecutor = executorService; } @Override public synchronized I get() { - long beforeGetTimeMilis = System.currentTimeMillis(); startLoad(); try { - I value = mFutureValue.get(); - long afterGetTimeMilis = System.currentTimeMillis(); - long difference = afterGetTimeMilis - beforeGetTimeMilis; - // If it takes more than 0.5 seconds it seemed to have performance issue? - if (difference > 500) { - Log.w(TAG, getType().getName() + " takes " + difference + " ms"); + while (!mFutureValue.isDone()) { + // try to run next task to avoid deadlock due to limited thread size + Runnable nextTask = mThreadPoolExecutor.getQueue().poll(); + if (nextTask != null) { + nextTask.run(); + } } - return value; + return mFutureValue.get(); } catch (Exception e) { Log.e(TAG, getType().getName() + " throws exception with message: " + e.getMessage()); throw new RuntimeException(e); @@ -39,7 +38,7 @@ public synchronized I get() { public synchronized void startLoad() { if (mFutureValue == null) { - mFutureValue = mExecutorService.submit(() -> getProviderValue().get()); + mFutureValue = mThreadPoolExecutor.submit(() -> getProviderValue().get()); } } } diff --git a/provider/src/test/java/m/co/rh/id/aprovider/ProviderUnitTest.java b/provider/src/test/java/m/co/rh/id/aprovider/ProviderUnitTest.java index 428156b..e453885 100644 --- a/provider/src/test/java/m/co/rh/id/aprovider/ProviderUnitTest.java +++ b/provider/src/test/java/m/co/rh/id/aprovider/ProviderUnitTest.java @@ -23,7 +23,10 @@ import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import m.co.rh.id.aprovider.test.IServiceA; @@ -550,7 +553,9 @@ public void provides(Context context, ProviderRegistry providerRegistry, Provide public void dispose(Context context, Provider provider) { // leave blank } - }, mockHandler, Executors.newSingleThreadExecutor()); + }, mockHandler, new ThreadPoolExecutor(1, 1, + 0L, TimeUnit.MILLISECONDS, + new LinkedBlockingQueue<>())); CountDownLatch testCountDownLatch = new CountDownLatch(1); AtomicReference atomicReference = new AtomicReference<>();