Sanitize the usage of executors.
There should be 1 executor each for static and dynamic language models. This prevents too many dynamic LM updates from running in parallel, competing for resources. Change-Id: I8ec439e0ea2d92fba275bc20a0b8c9193346a0c1main
parent
95711bfcee
commit
0b03f13cab
|
@ -23,22 +23,21 @@ import android.os.SystemClock;
|
|||
import android.provider.ContactsContract.Contacts;
|
||||
import android.util.Log;
|
||||
|
||||
import com.android.inputmethod.annotations.UsedForTesting;
|
||||
import com.android.inputmethod.latin.ContactsManager.ContactsChangedListener;
|
||||
import com.android.inputmethod.latin.utils.ExecutorUtils;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.concurrent.ExecutorService;
|
||||
|
||||
/**
|
||||
* A content observer that listens to updates to content provider {@link Contacts.CONTENT_URI}.
|
||||
* A content observer that listens to updates to content provider {@link Contacts#CONTENT_URI}.
|
||||
*/
|
||||
// TODO:add test
|
||||
public class ContactsContentObserver {
|
||||
public class ContactsContentObserver implements Runnable {
|
||||
private static final String TAG = ContactsContentObserver.class.getSimpleName();
|
||||
private static final boolean DEBUG = false;
|
||||
|
||||
private ContentObserver mObserver;
|
||||
private ContentObserver mContentObserver;
|
||||
private ContactsChangedListener mContactsChangedListener;
|
||||
|
||||
private final Context mContext;
|
||||
private final ContactsManager mManager;
|
||||
|
@ -52,29 +51,27 @@ public class ContactsContentObserver {
|
|||
if (DEBUG) {
|
||||
Log.d(TAG, "Registered Contacts Content Observer");
|
||||
}
|
||||
mObserver = new ContentObserver(null /* handler */) {
|
||||
mContactsChangedListener = listener;
|
||||
mContentObserver = new ContentObserver(null /* handler */) {
|
||||
@Override
|
||||
public void onChange(boolean self) {
|
||||
getBgExecutor().execute(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
if (haveContentsChanged()) {
|
||||
if (DEBUG) {
|
||||
Log.d(TAG, "Contacts have changed; notifying listeners");
|
||||
}
|
||||
listener.onContactsChange();
|
||||
}
|
||||
}
|
||||
});
|
||||
// TODO(zivkovic): Limit the queue to 1 instance of ContactsContentObserver.
|
||||
ExecutorUtils.getExecutorForDynamicLanguageModelUpdate()
|
||||
.execute(ContactsContentObserver.this);
|
||||
}
|
||||
};
|
||||
final ContentResolver contentResolver = mContext.getContentResolver();
|
||||
contentResolver.registerContentObserver(Contacts.CONTENT_URI, true, mObserver);
|
||||
contentResolver.registerContentObserver(Contacts.CONTENT_URI, true, mContentObserver);
|
||||
}
|
||||
|
||||
@UsedForTesting
|
||||
private ExecutorService getBgExecutor() {
|
||||
return ExecutorUtils.getExecutor("Check Contacts");
|
||||
@Override
|
||||
public void run() {
|
||||
if (haveContentsChanged()) {
|
||||
if (DEBUG) {
|
||||
Log.d(TAG, "Contacts have changed; notifying listeners");
|
||||
}
|
||||
mContactsChangedListener.onContactsChange();
|
||||
}
|
||||
}
|
||||
|
||||
private boolean haveContentsChanged() {
|
||||
|
@ -105,6 +102,6 @@ public class ContactsContentObserver {
|
|||
}
|
||||
|
||||
public void unregister() {
|
||||
mContext.getContentResolver().unregisterContentObserver(mObserver);
|
||||
mContext.getContentResolver().unregisterContentObserver(mContentObserver);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -443,7 +443,7 @@ public class DictionaryFacilitatorImpl implements DictionaryFacilitator {
|
|||
final Locale[] locales, final DictionaryInitializationListener listener) {
|
||||
final CountDownLatch latchForWaitingLoadingMainDictionary = new CountDownLatch(1);
|
||||
mLatchForWaitingLoadingMainDictionaries = latchForWaitingLoadingMainDictionary;
|
||||
ExecutorUtils.getExecutor("InitializeBinaryDictionary").execute(new Runnable() {
|
||||
ExecutorUtils.getExecutorForStaticLanguageModelUpdate().execute(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
doReloadUninitializedMainDictionaries(
|
||||
|
|
|
@ -164,12 +164,11 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
|
|||
}
|
||||
|
||||
private void asyncExecuteTaskWithWriteLock(final Runnable task) {
|
||||
asyncExecuteTaskWithLock(mLock.writeLock(), mDictName /* executorName */, task);
|
||||
asyncExecuteTaskWithLock(mLock.writeLock(), task);
|
||||
}
|
||||
|
||||
private static void asyncExecuteTaskWithLock(final Lock lock, final String executorName,
|
||||
final Runnable task) {
|
||||
ExecutorUtils.getExecutor(executorName).execute(new Runnable() {
|
||||
private static void asyncExecuteTaskWithLock(final Lock lock, final Runnable task) {
|
||||
ExecutorUtils.getExecutorForDynamicLanguageModelUpdate().execute(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
lock.lock();
|
||||
|
@ -663,7 +662,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
|
|||
final String dictName = mDictName;
|
||||
final File dictFile = mDictFile;
|
||||
final AsyncResultHolder<DictionaryStats> result = new AsyncResultHolder<>();
|
||||
asyncExecuteTaskWithLock(mLock.readLock(), dictName /* executorName */, new Runnable() {
|
||||
asyncExecuteTaskWithLock(mLock.readLock(), new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
final BinaryDictionary binaryDictionary = getBinaryDictionary();
|
||||
|
@ -714,7 +713,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
|
|||
reloadDictionaryIfRequired();
|
||||
final String tag = TAG;
|
||||
final String dictName = mDictName;
|
||||
asyncExecuteTaskWithLock(mLock.readLock(), "dumpAllWordsForDebug", new Runnable() {
|
||||
asyncExecuteTaskWithLock(mLock.readLock(), new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
Log.d(tag, "Dump dictionary: " + dictName + " for " + mLocale);
|
||||
|
@ -752,7 +751,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
|
|||
public WordProperty[] getWordPropertiesForSyncing() {
|
||||
reloadDictionaryIfRequired();
|
||||
final AsyncResultHolder<WordProperty[]> result = new AsyncResultHolder<>();
|
||||
asyncExecuteTaskWithLock(mLock.readLock(), "sync-read", new Runnable() {
|
||||
asyncExecuteTaskWithLock(mLock.readLock(), new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
final ArrayList<WordProperty> wordPropertyList = new ArrayList<>();
|
||||
|
|
|
@ -26,14 +26,13 @@ import android.util.Log;
|
|||
|
||||
import com.android.inputmethod.annotations.UsedForTesting;
|
||||
import com.android.inputmethod.latin.common.LocaleUtils;
|
||||
import com.android.inputmethod.latin.utils.ExecutorUtils;
|
||||
|
||||
import java.io.Closeable;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.Locale;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.ScheduledExecutorService;
|
||||
import java.util.concurrent.ScheduledFuture;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
|
@ -82,12 +81,6 @@ public class UserDictionaryLookup implements Closeable {
|
|||
|
||||
private final ContentResolver mResolver;
|
||||
|
||||
/**
|
||||
* Executor on which to perform the initial load and subsequent reloads (after a delay).
|
||||
*/
|
||||
private final ScheduledExecutorService mLoadExecutor =
|
||||
Executors.newSingleThreadScheduledExecutor();
|
||||
|
||||
/**
|
||||
* Runnable that calls loadUserDictionary().
|
||||
*/
|
||||
|
@ -150,7 +143,8 @@ public class UserDictionaryLookup implements Closeable {
|
|||
}
|
||||
|
||||
// Schedule a new reload after RELOAD_DELAY_MS.
|
||||
mReloadFuture = mLoadExecutor.schedule(mLoader, RELOAD_DELAY_MS, TimeUnit.MILLISECONDS);
|
||||
mReloadFuture = ExecutorUtils.getExecutorForDynamicLanguageModelUpdate().schedule(
|
||||
mLoader, RELOAD_DELAY_MS, TimeUnit.MILLISECONDS);
|
||||
}
|
||||
}
|
||||
private final ContentObserver mObserver = new UserDictionaryContentObserver();
|
||||
|
@ -192,7 +186,8 @@ public class UserDictionaryLookup implements Closeable {
|
|||
// Schedule the initial load to run immediately. It's possible that the first call to
|
||||
// isValidWord occurs before the dictionary has actually loaded, so it should not
|
||||
// assume that the dictionary has been loaded.
|
||||
mLoadExecutor.schedule(mLoader, 0, TimeUnit.MILLISECONDS);
|
||||
ExecutorUtils.getExecutorForDynamicLanguageModelUpdate().schedule(
|
||||
mLoader, 0, TimeUnit.MILLISECONDS);
|
||||
|
||||
// Register the observer to be notified on changes to the UserDictionary and all individual
|
||||
// items.
|
||||
|
@ -236,9 +231,6 @@ public class UserDictionaryLookup implements Closeable {
|
|||
Log.d(TAG, "Close called (no pun intended), cleaning up executor and observer");
|
||||
}
|
||||
if (mIsClosed.compareAndSet(false, true)) {
|
||||
// Shut down the load executor.
|
||||
mLoadExecutor.shutdown();
|
||||
|
||||
// Unregister the content observer.
|
||||
mResolver.unregisterContentObserver(mObserver);
|
||||
}
|
||||
|
@ -342,8 +334,7 @@ public class UserDictionaryLookup implements Closeable {
|
|||
if (DEBUG) {
|
||||
Log.d(TAG, "Loading UserDictionary");
|
||||
}
|
||||
HashMap<String, ArrayList<Locale>> dictWords =
|
||||
new HashMap<String, ArrayList<Locale>>();
|
||||
HashMap<String, ArrayList<Locale>> dictWords = new HashMap<>();
|
||||
// Load the UserDictionary. Request that items be returned in the default sort order
|
||||
// for UserDictionary, which is by frequency.
|
||||
Cursor cursor = mResolver.query(UserDictionary.Words.CONTENT_URI,
|
||||
|
@ -413,7 +404,7 @@ public class UserDictionaryLookup implements Closeable {
|
|||
Log.d(TAG, "Word [" + dictWord +
|
||||
"] not seen for other locales, creating new entry");
|
||||
}
|
||||
dictLocales = new ArrayList<Locale>();
|
||||
dictLocales = new ArrayList<>();
|
||||
dictWords.put(dictWord, dictLocales);
|
||||
}
|
||||
// Append the locale to the list of locales this word is in.
|
||||
|
|
|
@ -16,10 +16,12 @@
|
|||
|
||||
package com.android.inputmethod.latin.utils;
|
||||
|
||||
import android.util.Log;
|
||||
|
||||
import com.android.inputmethod.annotations.UsedForTesting;
|
||||
|
||||
import java.lang.Thread.UncaughtExceptionHandler;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.ExecutorService;
|
||||
import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.ScheduledExecutorService;
|
||||
import java.util.concurrent.ThreadFactory;
|
||||
|
@ -28,33 +30,49 @@ import java.util.concurrent.ThreadFactory;
|
|||
* Utilities to manage executors.
|
||||
*/
|
||||
public class ExecutorUtils {
|
||||
static final ConcurrentHashMap<String, ScheduledExecutorService> sExecutorMap =
|
||||
|
||||
private static final String STATIC_LANGUAGE_MODEL_UPDATE = "StaticLanguageModelUpdate";
|
||||
private static final String DYNAMIC_LANGUAGE_MODEL_UPDATE = "DynamicLanguageModelUpdate";
|
||||
|
||||
private static final ConcurrentHashMap<String, ScheduledExecutorService> sExecutorMap =
|
||||
new ConcurrentHashMap<>();
|
||||
|
||||
private static class ThreadFactoryWithId implements ThreadFactory {
|
||||
private final String mId;
|
||||
@UsedForTesting
|
||||
private static ScheduledExecutorService sExecutorServiceForTests;
|
||||
|
||||
public ThreadFactoryWithId(final String id) {
|
||||
mId = id;
|
||||
}
|
||||
@UsedForTesting
|
||||
public static void setExecutorServiceForTests(
|
||||
final ScheduledExecutorService executorServiceForTests) {
|
||||
sExecutorServiceForTests = executorServiceForTests;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Thread newThread(final Runnable r) {
|
||||
return new Thread(r, "Executor - " + mId);
|
||||
}
|
||||
/**
|
||||
* @return scheduled executor service used to update static language models
|
||||
*/
|
||||
public static ScheduledExecutorService getExecutorForStaticLanguageModelUpdate() {
|
||||
return getExecutor(STATIC_LANGUAGE_MODEL_UPDATE);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return scheduled executor service used to update dynamic language models
|
||||
*/
|
||||
public static ScheduledExecutorService getExecutorForDynamicLanguageModelUpdate() {
|
||||
return getExecutor(DYNAMIC_LANGUAGE_MODEL_UPDATE);
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the executor for the given id.
|
||||
*/
|
||||
public static ScheduledExecutorService getExecutor(final String id) {
|
||||
private static ScheduledExecutorService getExecutor(final String id) {
|
||||
if (sExecutorServiceForTests != null) {
|
||||
return sExecutorServiceForTests;
|
||||
}
|
||||
ScheduledExecutorService executor = sExecutorMap.get(id);
|
||||
if (executor == null) {
|
||||
synchronized (sExecutorMap) {
|
||||
executor = sExecutorMap.get(id);
|
||||
if (executor == null) {
|
||||
executor = Executors.newSingleThreadScheduledExecutor(
|
||||
new ThreadFactoryWithId(id));
|
||||
executor = Executors.newSingleThreadScheduledExecutor(new ExecutorFactory(id));
|
||||
sExecutorMap.put(id, executor);
|
||||
}
|
||||
}
|
||||
|
@ -69,14 +87,42 @@ public class ExecutorUtils {
|
|||
public static void shutdownAllExecutors() {
|
||||
synchronized (sExecutorMap) {
|
||||
for (final ScheduledExecutorService executor : sExecutorMap.values()) {
|
||||
executor.execute(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
executor.shutdown();
|
||||
sExecutorMap.remove(executor);
|
||||
}
|
||||
});
|
||||
executor.execute(new ExecutorShutdown(executor));
|
||||
}
|
||||
sExecutorMap.clear();
|
||||
}
|
||||
}
|
||||
|
||||
private static class ExecutorFactory implements ThreadFactory {
|
||||
private final String mThreadName;
|
||||
|
||||
public ExecutorFactory(final String threadName) {
|
||||
mThreadName = threadName;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Thread newThread(final Runnable runnable) {
|
||||
Thread thread = new Thread(runnable, mThreadName);
|
||||
thread.setUncaughtExceptionHandler(new UncaughtExceptionHandler() {
|
||||
@Override
|
||||
public void uncaughtException(Thread thread, Throwable ex) {
|
||||
Log.w(mThreadName + "-" + runnable.getClass().getSimpleName(), ex);
|
||||
}
|
||||
});
|
||||
return thread;
|
||||
}
|
||||
}
|
||||
|
||||
private static class ExecutorShutdown implements Runnable {
|
||||
private final ScheduledExecutorService mExecutor;
|
||||
|
||||
public ExecutorShutdown(final ScheduledExecutorService executor) {
|
||||
mExecutor = executor;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void run() {
|
||||
mExecutor.shutdown();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -25,18 +25,17 @@ import java.util.concurrent.TimeUnit;
|
|||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
/**
|
||||
* Unit tests for ExecutorUtils.
|
||||
* Unit tests for {@link ExecutorUtils}.
|
||||
*/
|
||||
@MediumTest
|
||||
public class ExecutorUtilsTests extends AndroidTestCase {
|
||||
private static final String TAG = ExecutorUtilsTests.class.getSimpleName();
|
||||
|
||||
private static final String TEST_EXECUTOR_ID = "test";
|
||||
private static final int NUM_OF_TASKS = 10;
|
||||
private static final int DELAY_FOR_WAITING_TASKS_MILLISECONDS = 500;
|
||||
|
||||
public void testExecute() {
|
||||
final ExecutorService executor = ExecutorUtils.getExecutor(TEST_EXECUTOR_ID);
|
||||
final ExecutorService executor = ExecutorUtils.getExecutorForDynamicLanguageModelUpdate();
|
||||
final AtomicInteger v = new AtomicInteger(0);
|
||||
for (int i = 0; i < NUM_OF_TASKS; ++i) {
|
||||
executor.execute(new Runnable() {
|
||||
|
|
Loading…
Reference in New Issue