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
main
Yuichiro Hanada 2013-09-27 11:13:22 +09:00 committed by Ken Wakasa
parent ef63947f7d
commit ceb364c041
4 changed files with 46 additions and 47 deletions

View File

@ -727,7 +727,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
holder.set(mBinaryDictionary.isValidWord(word)); holder.set(mBinaryDictionary.isValidWord(word));
} else { } else {
holder.set(((DynamicPersonalizationDictionaryWriter) mDictionaryWriter) holder.set(((DynamicPersonalizationDictionaryWriter) mDictionaryWriter)
.isInDictionaryForTests(word)); .isInBigramListForTests(word));
} }
} }
} }

View File

@ -79,7 +79,7 @@ public class DynamicPersonalizationDictionaryWriter extends AbstractDictionaryWr
public void addUnigramWord(final String word, final String shortcutTarget, final int frequency, public void addUnigramWord(final String word, final String shortcutTarget, final int frequency,
final boolean isNotAWord) { final boolean isNotAWord) {
if (mBigramList.size() > mMaxHistoryBigrams * 2) { 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; return;
} }
mExpandableDictionary.addWord(word, shortcutTarget, frequency); 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, public void addBigramWords(final String word0, final String word1, final int frequency,
final boolean isValid, final long lastModifiedTime) { final boolean isValid, final long lastModifiedTime) {
if (mBigramList.size() > mMaxHistoryBigrams * 2) { 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; return;
} }
if (lastModifiedTime > 0) { if (lastModifiedTime > 0) {
@ -176,8 +176,8 @@ public class DynamicPersonalizationDictionaryWriter extends AbstractDictionaryWr
} }
@UsedForTesting @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 // 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);
} }
} }

View File

@ -16,8 +16,11 @@
package com.android.inputmethod.latin.utils; package com.android.inputmethod.latin.utils;
import java.util.ArrayDeque;
import java.util.Queue; 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. * An object that executes submitted tasks using a thread.
@ -27,19 +30,20 @@ public class PrioritizedSerialExecutor {
private final Object mLock = new Object(); 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> mTasks;
private final Queue<Runnable> mPrioritizedTasks; private final Queue<Runnable> mPrioritizedTasks;
private boolean mIsShutdown; private boolean mIsShutdown;
private final ThreadPoolExecutor mThreadPoolExecutor;
// The task which is running now. // The task which is running now.
private Runnable mActive; private Runnable mActive;
public PrioritizedSerialExecutor() { public PrioritizedSerialExecutor() {
mTasks = new ArrayDeque<Runnable>(TASK_QUEUE_CAPACITY); mTasks = new ConcurrentLinkedQueue<Runnable>();
mPrioritizedTasks = new ArrayDeque<Runnable>(TASK_QUEUE_CAPACITY); mPrioritizedTasks = new ConcurrentLinkedQueue<Runnable>();
mIsShutdown = false; 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) { public void execute(final Runnable r) {
synchronized(mLock) { synchronized(mLock) {
if (!mIsShutdown) { if (!mIsShutdown) {
mTasks.offer(r); mTasks.offer(new Runnable() {
@Override
public void run() {
try {
r.run();
} finally {
scheduleNext();
}
}
});
if (mActive == null) { if (mActive == null) {
scheduleNext(); scheduleNext();
} }
@ -74,7 +87,16 @@ public class PrioritizedSerialExecutor {
public void executePrioritized(final Runnable r) { public void executePrioritized(final Runnable r) {
synchronized(mLock) { synchronized(mLock) {
if (!mIsShutdown) { if (!mIsShutdown) {
mPrioritizedTasks.offer(r); mPrioritizedTasks.offer(new Runnable() {
@Override
public void run() {
try {
r.run();
} finally {
scheduleNext();
}
}
});
if (mActive == null) { if (mActive == null) {
scheduleNext(); scheduleNext();
} }
@ -82,37 +104,19 @@ public class PrioritizedSerialExecutor {
} }
} }
private boolean fetchNextTasks() { private boolean fetchNextTasksLocked() {
synchronized(mLock) {
mActive = mPrioritizedTasks.poll(); mActive = mPrioritizedTasks.poll();
if (mActive == null) { if (mActive == null) {
mActive = mTasks.poll(); mActive = mTasks.poll();
} }
return mActive != null; return mActive != null;
} }
}
private void scheduleNext() { private void scheduleNext() {
synchronized(mLock) { synchronized(mLock) {
if (!fetchNextTasks()) { if (fetchNextTasksLocked()) {
return; 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();
} }
} }

View File

@ -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. * or not.
*/ */
private void addAndWriteRandomWords(final String testFilenameSuffix, final int numberOfWords, 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 List<String> words = generateWords(numberOfWords, random);
final UserHistoryPredictionDictionary dict = final UserHistoryPredictionDictionary dict =
PersonalizationHelper.getUserHistoryPredictionDictionary(getContext(), PersonalizationHelper.getUserHistoryPredictionDictionary(getContext(),
testFilenameSuffix /* locale */, mPrefs); testFilenameSuffix /* locale */, mPrefs);
// Add random words to the user history dictionary. // Add random words to the user history dictionary.
addToDict(dict, words); addToDict(dict, words);
if (checksContents) { if (checkContents) {
try { try {
Thread.sleep(TimeUnit.MILLISECONDS.convert(5L, TimeUnit.SECONDS)); Thread.sleep(TimeUnit.MILLISECONDS.convert(5L, TimeUnit.SECONDS));
} catch (InterruptedException e) { } catch (InterruptedException e) {
} }
// Limit word count to check when using a Java on memory dictionary. for (int i = 0; i < numberOfWords; ++i) {
final int wordCountToCheck =
ExpandableBinaryDictionary.ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE ?
numberOfWords : 10;
for (int i = 0; i < wordCountToCheck; ++i) {
final String word = words.get(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)); assertTrue(dict.isInDictionaryForTests(word));
} }
} }