From 9d4f433e4963eebd63a71ae9fda4dcbf75c66dad Mon Sep 17 00:00:00 2001 From: Keisuke Kuroyanagi Date: Fri, 29 Nov 2013 20:35:00 +0900 Subject: [PATCH] Fix bugs of GC. - All PtNodes were treated as terminal during GC. - Cannot delete Ver4 PtNode. Bug: 11073222 Change-Id: I26e012cc7154b6267e3499559b457dfee601748f --- ...namic_patricia_trie_gc_event_listeners.cpp | 3 +- .../v3/dynamic_patricia_trie_node_writer.cpp | 3 +- .../v4/content/probability_dict_content.cpp | 3 +- .../terminal_position_lookup_table.cpp | 3 -- .../v4/ver4_patricia_trie_node_writer.cpp | 2 +- .../v4/ver4_patricia_trie_writing_helper.cpp | 33 +++++++++---------- 6 files changed, 20 insertions(+), 27 deletions(-) diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_gc_event_listeners.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_gc_event_listeners.cpp index c582a6e76..22d4f7ac8 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_gc_event_listeners.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_gc_event_listeners.cpp @@ -127,8 +127,7 @@ bool DynamicPatriciaTrieGcEventListeners::TraversePolicyToPlaceAndWriteValidPtNo ptNodeParams->getHeadPos(), writingPos)); mValidPtNodeCount++; // Writes current PtNode. - return mPtNodeWriter->writeNewTerminalPtNodeAndAdvancePosition(ptNodeParams, - 0 /* timestamp */, &writingPos); + return mPtNodeWriter->writePtNodeAndAdvancePosition(ptNodeParams, &writingPos); } bool DynamicPatriciaTrieGcEventListeners::TraversePolicyToUpdateAllPositionFields diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_node_writer.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_node_writer.cpp index d925a7e06..22255431f 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_node_writer.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v3/dynamic_patricia_trie_node_writer.cpp @@ -174,8 +174,7 @@ bool DynamicPatriciaTrieNodeWriter::addNewBigramEntry( // Then, Mark as the PtNode having bigram list in the flags. const PatriciaTrieReadingUtils::NodeFlags updatedFlags = PatriciaTrieReadingUtils::createAndGetFlags(newPtNodeParams.isBlacklisted(), - newPtNodeParams.isNotAWord(), - newPtNodeParams.getProbability() != NOT_A_PROBABILITY, + newPtNodeParams.isNotAWord(), newPtNodeParams.isTerminal(), newPtNodeParams.getShortcutPos() != NOT_A_DICT_POS, true /* hasBigrams */, newPtNodeParams.getCodePointCount() > 1, CHILDREN_POSITION_FIELD_SIZE); writingPos = newNodePos; diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/probability_dict_content.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/probability_dict_content.cpp index 0222080d1..473ab0d9f 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/probability_dict_content.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/probability_dict_content.cpp @@ -26,9 +26,8 @@ namespace latinime { void ProbabilityDictContent::getProbabilityEntry(const int terminalId, ProbabilityEntry *const outProbabilityEntry) const { if (terminalId < 0 || terminalId >= mSize) { + // This method can be called with invalid terminal id during GC. outProbabilityEntry->setProbability(0 /* flags */, NOT_A_PROBABILITY); - AKLOGE("Terminal id (%d) is not in the probability dict content. mSize: %d", terminalId, - mSize); return; } const BufferWithExtendableBuffer *const buffer = getBuffer(); diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/terminal_position_lookup_table.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/terminal_position_lookup_table.cpp index a89e242ba..c38aeb4ca 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/terminal_position_lookup_table.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/content/terminal_position_lookup_table.cpp @@ -44,9 +44,6 @@ bool TerminalPositionLookupTable::setTerminalPtNodePosition( } mSize++; } - if (terminalPtNodePos == NOT_A_DICT_POS) { - return true; - } const int terminalPos = (terminalPtNodePos != NOT_A_DICT_POS) ? terminalPtNodePos + mHeaderRegionSize : Ver4DictConstants::NOT_A_TERMINAL_ADDRESS; return getWritableBuffer()->writeUint(terminalPos, diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_node_writer.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_node_writer.cpp index 145eeb08f..9aa6e60c5 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_node_writer.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_node_writer.cpp @@ -51,7 +51,7 @@ bool Ver4PatriciaTrieNodeWriter::markPtNodeAsDeleted( &writingPos)) { return false; } - if (toBeUpdatedPtNodeParams->getTerminalId() != NOT_A_DICT_POS) { + if (toBeUpdatedPtNodeParams->isTerminal()) { // The PtNode is a terminal. Delete entry from the terminal position lookup table. return mBuffers->getUpdatableTerminalPositionLookupTable()->setTerminalPtNodePosition( toBeUpdatedPtNodeParams->getTerminalId(), NOT_A_DICT_POS /* ptNodePos */); diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_writing_helper.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_writing_helper.cpp index a7b729b94..e8368af27 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_writing_helper.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/ver4_patricia_trie_writing_helper.cpp @@ -143,29 +143,12 @@ bool Ver4PatriciaTrieWritingHelper::runGC(const int rootPtNodeArrayPos, Ver4PatriciaTrieNodeWriter newPtNodeWriter(buffersToWrite->getWritableTrieBuffer(), buffersToWrite, &newPtNodeReader, &newBigramPolicy, &newShortcutPolicy, false /* needsToDecayWhenUpdating */); - - DynamicPatriciaTrieReadingHelper newDictReadingHelper(buffersToWrite->getTrieBuffer(), - &newPtNodeReader); - newDictReadingHelper.initWithPtNodeArrayPos(rootPtNodeArrayPos); - DynamicPatriciaTrieGcEventListeners::TraversePolicyToUpdateAllPositionFields - traversePolicyToUpdateAllPositionFields(&newPtNodeWriter, &dictPositionRelocationMap); - if (!newDictReadingHelper.traverseAllPtNodesInPtNodeArrayLevelPreorderDepthFirstManner( - &traversePolicyToUpdateAllPositionFields)) { - return false; - } - // Re-assign terminal IDs for valid terminal PtNodes. TerminalPositionLookupTable::TerminalIdMap terminalIdMap; if(!buffersToWrite->getUpdatableTerminalPositionLookupTable()->runGCTerminalIds( &terminalIdMap)) { return false; } - TraversePolicyToUpdateAllTerminalIds traversePolicyToUpdateAllTerminalIds(&newPtNodeWriter, - &terminalIdMap); - if (!newDictReadingHelper.traverseAllPtNodesInPostorderDepthFirstManner( - &traversePolicyToUpdateAllTerminalIds)) { - return false; - } // Run GC for probability dict content. if (!buffersToWrite->getUpdatableProbabilityDictContent()->runGC(&terminalIdMap, mBuffers->getProbabilityDictContent())) { @@ -181,6 +164,22 @@ bool Ver4PatriciaTrieWritingHelper::runGC(const int rootPtNodeArrayPos, mBuffers->getShortcutDictContent())) { return false; } + DynamicPatriciaTrieReadingHelper newDictReadingHelper(buffersToWrite->getTrieBuffer(), + &newPtNodeReader); + newDictReadingHelper.initWithPtNodeArrayPos(rootPtNodeArrayPos); + DynamicPatriciaTrieGcEventListeners::TraversePolicyToUpdateAllPositionFields + traversePolicyToUpdateAllPositionFields(&newPtNodeWriter, &dictPositionRelocationMap); + if (!newDictReadingHelper.traverseAllPtNodesInPtNodeArrayLevelPreorderDepthFirstManner( + &traversePolicyToUpdateAllPositionFields)) { + return false; + } + newDictReadingHelper.initWithPtNodeArrayPos(rootPtNodeArrayPos); + TraversePolicyToUpdateAllTerminalIds traversePolicyToUpdateAllTerminalIds(&newPtNodeWriter, + &terminalIdMap); + if (!newDictReadingHelper.traverseAllPtNodesInPostorderDepthFirstManner( + &traversePolicyToUpdateAllTerminalIds)) { + return false; + } *outUnigramCount = traversePolicyToUpdateAllPositionFields.getUnigramCount(); return true; }