diff options
7 files changed, 67 insertions, 48 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/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_node_reader.cpp b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_node_reader.cpp index 456352c17..2fa3111d3 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_node_reader.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_node_reader.cpp @@ -26,7 +26,8 @@ namespace latinime { void DynamicPatriciaTrieNodeReader::fetchPtNodeInfoFromBufferAndProcessMovedPtNode( const int ptNodePos, const int maxCodePointCount, int *const outCodePoints) { if (ptNodePos < 0 || ptNodePos >= mBuffer->getTailPosition()) { - AKLOGE("Fetching PtNode info form invalid dictionary position: %d, dictionary size: %d", + // Reading invalid position because of bug or broken dictionary. + AKLOGE("Fetching PtNode info from invalid dictionary position: %d, dictionary size: %d", ptNodePos, mBuffer->getTailPosition()); ASSERT(false); invalidatePtNodeInfo(); diff --git a/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.cpp b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.cpp index f4a2ef389..601ee663b 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.cpp @@ -155,6 +155,15 @@ bool DynamicPatriciaTrieReadingHelper::traverseAllPtNodesInPtNodeArrayLevelPreor // Read node array size and process empty node arrays. Nodes and arrays are counted up in this // method to avoid an infinite loop. void DynamicPatriciaTrieReadingHelper::nextPtNodeArray() { + if (mReadingState.mPos < 0 || mReadingState.mPos >= mBuffer->getTailPosition()) { + // Reading invalid position because of a bug or a broken dictionary. + AKLOGE("Reading PtNode array info from invalid dictionary position: %d, dict size: %d", + mReadingState.mPos, mBuffer->getTailPosition()); + ASSERT(false); + mIsError = true; + mReadingState.mPos = NOT_A_DICT_POS; + return; + } mReadingState.mPosOfLastPtNodeArrayHead = mReadingState.mPos; const bool usesAdditionalBuffer = mBuffer->isInAdditionalBuffer(mReadingState.mPos); const uint8_t *const dictBuf = mBuffer->getBuffer(usesAdditionalBuffer); @@ -191,6 +200,15 @@ void DynamicPatriciaTrieReadingHelper::nextPtNodeArray() { // Follow the forward link and read the next node array if exists. void DynamicPatriciaTrieReadingHelper::followForwardLink() { + if (mReadingState.mPos < 0 || mReadingState.mPos >= mBuffer->getTailPosition()) { + // Reading invalid position because of bug or broken dictionary. + AKLOGE("Reading forward link from invalid dictionary position: %d, dict size: %d", + mReadingState.mPos, mBuffer->getTailPosition()); + ASSERT(false); + mIsError = true; + mReadingState.mPos = NOT_A_DICT_POS; + return; + } const bool usesAdditionalBuffer = mBuffer->isInAdditionalBuffer(mReadingState.mPos); const uint8_t *const dictBuf = mBuffer->getBuffer(usesAdditionalBuffer); if (usesAdditionalBuffer) { diff --git a/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.h b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.h index b033eee05..154590fbd 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.h +++ b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_reading_helper.h @@ -240,6 +240,7 @@ class DynamicPatriciaTrieReadingHelper { static const int MAX_NODE_ARRAY_COUNT_TO_AVOID_INFINITE_LOOP; static const size_t MAX_READING_STATE_STACK_SIZE; + // TODO: Introduce error code to track what caused the error. bool mIsError; ReadingState mReadingState; const BufferWithExtendableBuffer *const mBuffer; 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)); } } |