Fix looping logic bugs.

shiftOut() is getting called once too often through these for loops.

Change-Id: I9a68b49e6cc1469bcddd673ab1567e238cf192b8
main
Kurt Partridge 2013-04-19 16:02:02 -07:00
parent 104bb70c65
commit 8064c669fe
2 changed files with 31 additions and 25 deletions

View File

@ -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() {

View File

@ -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<LogUnit> 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;
}
}