aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorYuichiro Hanada <yhanada@google.com>2013-09-27 11:13:22 +0900
committerKen Wakasa <kwakasa@google.com>2013-09-29 11:41:13 +0900
commitceb364c0411dbcb38f2bd94f1d2b54eef3b985c1 (patch)
tree77f2ddf49d14d458dc9f9e726ec14381204109df
parentef63947f7d10bcbb3237661ad480b430d4208833 (diff)
downloadlatinime-ceb364c0411dbcb38f2bd94f1d2b54eef3b985c1.tar.gz
latinime-ceb364c0411dbcb38f2bd94f1d2b54eef3b985c1.tar.xz
latinime-ceb364c0411dbcb38f2bd94f1d2b54eef3b985c1.zip
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
-rw-r--r--java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java2
-rw-r--r--java/src/com/android/inputmethod/latin/personalization/DynamicPersonalizationDictionaryWriter.java8
-rw-r--r--java/src/com/android/inputmethod/latin/utils/PrioritizedSerialExecutor.java70
-rw-r--r--tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java13
4 files changed, 46 insertions, 47 deletions
diff --git a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
index 183f12ad9..712520212 100644
--- a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
+++ b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
@@ -727,7 +727,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
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 0af028a9e..305088536 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 class DynamicPersonalizationDictionaryWriter extends AbstractDictionaryWr
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 class DynamicPersonalizationDictionaryWriter extends AbstractDictionaryWr
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 @@ public class DynamicPersonalizationDictionaryWriter extends AbstractDictionaryWr
}
@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 5dc0b5893..201a70d42 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 @@ public class PrioritizedSerialExecutor {
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 class PrioritizedSerialExecutor {
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 class PrioritizedSerialExecutor {
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 d605cdb84..06c427193 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 @@ public class UserHistoryDictionaryTests extends AndroidTestCase {
}
/**
- * @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));
}
}