[Performance] Optimize thread pool usage across all provider instances

This prevents ThreadPool threads to grow too large when using registerAsync
rh-id · Nov 19, 2021 · 37c624a · 37c624a
1 parent 2b7d763
commit 37c624a
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 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
@@ -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<ProviderModule> mModuleList;
     private List<LazyFutureProviderRegister> 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> I exactGet(Class<I> clazz) {
 
     @Override
     public <I> void getAsyncAndDo(Class<I> clazz, ProviderAction<I> actionOnMainThread) {
-        mExecutorService.execute(() -> {
+        mThreadPoolExecutor.execute(() -> {
             try {
                 I value = get(clazz);
                 mHandler.post(() -> actionOnMainThread.onSuccess(value));
@@ -177,7 +175,7 @@ public <I> void registerLazy(Class<I> clazz, ProviderValue<I> providerValue) {
 
     @Override
     public <I> void registerAsync(Class<I> clazz, ProviderValue<I> 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
@@ -2,35 +2,34 @@
 
 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
  */
 class LazyFutureProviderRegister<I> extends ProviderRegister<I> {
     private static final String TAG = "FutureProvider";
     private Future<I> mFutureValue;
-    private ExecutorService mExecutorService;
+    private ThreadPoolExecutor mThreadPoolExecutor;
 
-    public LazyFutureProviderRegister(Class<I> type, ProviderValue<I> providerValue, ExecutorService executorService) {
+    public LazyFutureProviderRegister(Class<I> type, ProviderValue<I> 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
@@ -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<Object> atomicReference = new AtomicReference<>();