Fix PrioritizedSerialExecutor.
It was possible that fetchNextTasks() would be called by multiple
threads concurrently.
If it happens, some tasks in the task queues might be ignored.
cherrypick of Idc81c43c45e382da3850cc55b9a42c281548d2a8
bug: 10957075
Change-Id: I41bf90dda15306ee879350f96a791d940ea31da1
diff --git a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
index 183f12a..7125202 100644
--- a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
+++ b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
@@ -727,7 +727,7 @@
holder.set(mBinaryDictionary.isValidWord(word));
} else {
holder.set(((DynamicPersonalizationDictionaryWriter) mDictionaryWriter)
- .isInDictionaryForTests(word));
+ .isInBigramListForTests(word));
}
}
}
diff --git a/java/src/com/android/inputmethod/latin/personalization/DynamicPersonalizationDictionaryWriter.java b/java/src/com/android/inputmethod/latin/personalization/DynamicPersonalizationDictionaryWriter.java
index 0af028a..3050885 100644
--- a/java/src/com/android/inputmethod/latin/personalization/DynamicPersonalizationDictionaryWriter.java
+++ b/java/src/com/android/inputmethod/latin/personalization/DynamicPersonalizationDictionaryWriter.java
@@ -79,7 +79,7 @@
public void addUnigramWord(final String word, final String shortcutTarget, final int frequency,
final boolean isNotAWord) {
if (mBigramList.size() > mMaxHistoryBigrams * 2) {
- // Too many entries: just stop adding new vocabrary and wait next refresh.
+ // Too many entries: just stop adding new vocabulary and wait next refresh.
return;
}
mExpandableDictionary.addWord(word, shortcutTarget, frequency);
@@ -90,7 +90,7 @@
public void addBigramWords(final String word0, final String word1, final int frequency,
final boolean isValid, final long lastModifiedTime) {
if (mBigramList.size() > mMaxHistoryBigrams * 2) {
- // Too many entries: just stop adding new vocabrary and wait next refresh.
+ // Too many entries: just stop adding new vocabulary and wait next refresh.
return;
}
if (lastModifiedTime > 0) {
@@ -176,8 +176,8 @@
}
@UsedForTesting
- public boolean isInDictionaryForTests(final String word) {
+ public boolean isInBigramListForTests(final String word) {
// TODO: Use native method to determine whether the word is in dictionary or not
- return mBigramList.containsKey(word);
+ return mBigramList.containsKey(word) || mBigramList.getBigrams(null).containsKey(word);
}
}
diff --git a/java/src/com/android/inputmethod/latin/utils/PrioritizedSerialExecutor.java b/java/src/com/android/inputmethod/latin/utils/PrioritizedSerialExecutor.java
index 5dc0b58..201a70d 100644
--- a/java/src/com/android/inputmethod/latin/utils/PrioritizedSerialExecutor.java
+++ b/java/src/com/android/inputmethod/latin/utils/PrioritizedSerialExecutor.java
@@ -16,8 +16,11 @@
package com.android.inputmethod.latin.utils;
-import java.util.ArrayDeque;
import java.util.Queue;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
/**
* An object that executes submitted tasks using a thread.
@@ -27,19 +30,20 @@
private final Object mLock = new Object();
- // The default value of capacities of task queues.
- private static final int TASK_QUEUE_CAPACITY = 1000;
private final Queue<Runnable> mTasks;
private final Queue<Runnable> mPrioritizedTasks;
private boolean mIsShutdown;
+ private final ThreadPoolExecutor mThreadPoolExecutor;
// The task which is running now.
private Runnable mActive;
public PrioritizedSerialExecutor() {
- mTasks = new ArrayDeque<Runnable>(TASK_QUEUE_CAPACITY);
- mPrioritizedTasks = new ArrayDeque<Runnable>(TASK_QUEUE_CAPACITY);
+ mTasks = new ConcurrentLinkedQueue<Runnable>();
+ mPrioritizedTasks = new ConcurrentLinkedQueue<Runnable>();
mIsShutdown = false;
+ mThreadPoolExecutor = new ThreadPoolExecutor(1 /* corePoolSize */, 1 /* maximumPoolSize */,
+ 0 /* keepAliveTime */, TimeUnit.MILLISECONDS, new ArrayBlockingQueue<Runnable>(1));
}
/**
@@ -59,7 +63,16 @@
public void execute(final Runnable r) {
synchronized(mLock) {
if (!mIsShutdown) {
- mTasks.offer(r);
+ mTasks.offer(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ r.run();
+ } finally {
+ scheduleNext();
+ }
+ }
+ });
if (mActive == null) {
scheduleNext();
}
@@ -74,45 +87,36 @@
public void executePrioritized(final Runnable r) {
synchronized(mLock) {
if (!mIsShutdown) {
- mPrioritizedTasks.offer(r);
- if (mActive == null) {
+ mPrioritizedTasks.offer(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ r.run();
+ } finally {
+ scheduleNext();
+ }
+ }
+ });
+ if (mActive == null) {
scheduleNext();
}
}
}
}
- private boolean fetchNextTasks() {
- synchronized(mLock) {
- mActive = mPrioritizedTasks.poll();
- if (mActive == null) {
- mActive = mTasks.poll();
- }
- return mActive != null;
+ private boolean fetchNextTasksLocked() {
+ mActive = mPrioritizedTasks.poll();
+ if (mActive == null) {
+ mActive = mTasks.poll();
}
+ return mActive != null;
}
private void scheduleNext() {
synchronized(mLock) {
- if (!fetchNextTasks()) {
- return;
+ if (fetchNextTasksLocked()) {
+ mThreadPoolExecutor.execute(mActive);
}
- new Thread(new Runnable() {
- @Override
- public void run() {
- try {
- do {
- synchronized(mLock) {
- if (mActive != null) {
- mActive.run();
- }
- }
- } while (fetchNextTasks());
- } finally {
- scheduleNext();
- }
- }
- }).start();
}
}
diff --git a/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java b/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java
index d605cdb..06c4271 100644
--- a/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java
+++ b/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java
@@ -84,29 +84,24 @@
}
/**
- * @param checksContents if true, checks whether written words are actually in the dictionary
+ * @param checkContents if true, checks whether written words are actually in the dictionary
* or not.
*/
private void addAndWriteRandomWords(final String testFilenameSuffix, final int numberOfWords,
- final Random random, final boolean checksContents) {
+ final Random random, final boolean checkContents) {
final List<String> words = generateWords(numberOfWords, random);
final UserHistoryPredictionDictionary dict =
PersonalizationHelper.getUserHistoryPredictionDictionary(getContext(),
testFilenameSuffix /* locale */, mPrefs);
// Add random words to the user history dictionary.
addToDict(dict, words);
- if (checksContents) {
+ if (checkContents) {
try {
Thread.sleep(TimeUnit.MILLISECONDS.convert(5L, TimeUnit.SECONDS));
} catch (InterruptedException e) {
}
- // Limit word count to check when using a Java on memory dictionary.
- final int wordCountToCheck =
- ExpandableBinaryDictionary.ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE ?
- numberOfWords : 10;
- for (int i = 0; i < wordCountToCheck; ++i) {
+ for (int i = 0; i < numberOfWords; ++i) {
final String word = words.get(i);
- // This may fail as long as we use tryLock on inserting the bigram words
assertTrue(dict.isInDictionaryForTests(word));
}
}