From 8064c669fe02cf90995dc82b1c67c8f434860dc5 Mon Sep 17 00:00:00 2001 From: Kurt Partridge Date: Fri, 19 Apr 2013 16:02:02 -0700 Subject: [PATCH] Fix looping logic bugs. shiftOut() is getting called once too often through these for loops. Change-Id: I9a68b49e6cc1469bcddd673ab1567e238cf192b8 --- .../inputmethod/research/FixedLogBuffer.java | 23 +++++++------ .../inputmethod/research/MainLogBuffer.java | 33 +++++++++++-------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/java/src/com/android/inputmethod/research/FixedLogBuffer.java b/java/src/com/android/inputmethod/research/FixedLogBuffer.java index 4249af544..8b64de8ae 100644 --- a/java/src/com/android/inputmethod/research/FixedLogBuffer.java +++ b/java/src/com/android/inputmethod/research/FixedLogBuffer.java @@ -65,6 +65,7 @@ public class FixedLogBuffer extends LogBuffer { final int numWordsIncoming = newLogUnit.getNumWords(); if (mNumActualWords >= mWordCapacity) { // Give subclass a chance to handle the buffer full condition by shifting out logUnits. + // TODO: Tell onBufferFull() how much space it needs to make to avoid forced eviction. onBufferFull(); // If still full, evict. if (mNumActualWords >= mWordCapacity) { @@ -119,21 +120,19 @@ public class FixedLogBuffer extends LogBuffer { /** * Remove LogUnits from the front of the LogBuffer until {@code numWords} have been removed. * - * If there are less than {@code numWords} word-containing {@link LogUnit}s, shifts out - * all {@code LogUnit}s in the buffer. + * If there are less than {@code numWords} in the buffer, shifts out all {@code LogUnit}s. * - * @param numWords the minimum number of word-containing {@link LogUnit}s to shift out - * @return the number of actual {@code LogUnit}s shifted out + * @param numWords the minimum number of words in {@link LogUnit}s to shift out + * @return the number of actual words LogUnit}s shifted out */ protected int shiftOutWords(final int numWords) { - int numWordContainingLogUnitsShiftedOut = 0; - for (LogUnit logUnit = shiftOut(); logUnit != null - && numWordContainingLogUnitsShiftedOut < numWords; logUnit = shiftOut()) { - if (logUnit.hasOneOrMoreWords()) { - numWordContainingLogUnitsShiftedOut += logUnit.getNumWords(); - } - } - return numWordContainingLogUnitsShiftedOut; + int numWordsShiftedOut = 0; + do { + final LogUnit logUnit = shiftOut(); + if (logUnit == null) break; + numWordsShiftedOut += logUnit.getNumWords(); + } while (numWordsShiftedOut < numWords); + return numWordsShiftedOut; } public void shiftOutAll() { diff --git a/java/src/com/android/inputmethod/research/MainLogBuffer.java b/java/src/com/android/inputmethod/research/MainLogBuffer.java index 42ef5d3b6..9bdedbf6d 100644 --- a/java/src/com/android/inputmethod/research/MainLogBuffer.java +++ b/java/src/com/android/inputmethod/research/MainLogBuffer.java @@ -190,22 +190,30 @@ public abstract class MainLogBuffer extends FixedLogBuffer { } protected final void publishLogUnitsAtFrontOfBuffer() { + // TODO: Refactor this method to require fewer passes through the LogUnits. Should really + // require only one pass. ArrayList logUnits = peekAtFirstNWords(N_GRAM_SIZE); if (isSafeNGram(logUnits, N_GRAM_SIZE)) { // Good n-gram at the front of the buffer. Publish it, disclosing details. publish(logUnits, true /* canIncludePrivateData */); shiftOutWords(N_GRAM_SIZE); mNumWordsUntilSafeToSample = mNumWordsBetweenNGrams; - } else { - // No good n-gram at front, and buffer is full. Shift out up through the first logUnit - // with associated words (or if there is none, all the existing logUnits). - logUnits.clear(); - for (LogUnit logUnit = shiftOut(); logUnit != null && !logUnit.hasOneOrMoreWords(); - logUnit = shiftOut()) { - logUnits.add(logUnit); - } - publish(logUnits, false /* canIncludePrivateData */); + return; } + // No good n-gram at front, and buffer is full. Shift out up through the first logUnit + // with associated words (or if there is none, all the existing logUnits). + logUnits.clear(); + LogUnit logUnit = shiftOut(); + while (logUnit != null) { + logUnits.add(logUnit); + final int numWords = logUnit.getNumWords(); + if (numWords > 0) { + mNumWordsUntilSafeToSample = Math.max(0, mNumWordsUntilSafeToSample - numWords); + break; + } + logUnit = shiftOut(); + } + publish(logUnits, false /* canIncludePrivateData */); } /** @@ -222,12 +230,11 @@ public abstract class MainLogBuffer extends FixedLogBuffer { @Override protected int shiftOutWords(final int numWords) { - final int numWordContainingLogUnitsShiftedOut = super.shiftOutWords(numWords); - mNumWordsUntilSafeToSample = Math.max(0, mNumWordsUntilSafeToSample - - numWordContainingLogUnitsShiftedOut); + final int numWordsShiftedOut = super.shiftOutWords(numWords); + mNumWordsUntilSafeToSample = Math.max(0, mNumWordsUntilSafeToSample - numWordsShiftedOut); if (DEBUG) { Log.d(TAG, "wordsUntilSafeToSample now at " + mNumWordsUntilSafeToSample); } - return numWordContainingLogUnitsShiftedOut; + return numWordsShiftedOut; } }