Merge "Rewrite BatteryStatsHistoryIterator logic for clarity and correctness" into main (ec78f2e8) · Commits · e / os / android_frameworks_base

core/java/com/android/internal/os/BatteryStatsHistory.java

+97 −93

Original line number	Diff line number	Diff line
		@@ -45,11 +45,13 @@ import com.android.internal.annotations.GuardedBy;
		import com.android.internal.annotations.VisibleForTesting;

		import java.io.PrintWriter;
		import java.util.ArrayDeque;
		import java.util.ArrayList;
		import java.util.ConcurrentModificationException;
		import java.util.HashMap;
		import java.util.List;
		import java.util.Map;
		import java.util.Queue;
		import java.util.concurrent.locks.ReentrantLock;

		/**
		@@ -202,15 +204,6 @@ public class BatteryStatsHistory {
		@Nullable
		BatteryHistoryFragment getLatestFragment();

		/**
		* Given a fragment, returns the earliest fragment that follows it whose monotonic
		* start time falls within the specified range. `startTimeMs` is inclusive, `endTimeMs`
		* is exclusive.
		*/
		@Nullable
		BatteryHistoryFragment getNextFragment(BatteryHistoryFragment current, long startTimeMs,
		long endTimeMs);

		/**
		* Acquires a lock on the entire store.
		*/
		@@ -268,6 +261,60 @@ public class BatteryStatsHistory {
		void reset();
		}

		class BatteryHistoryParcelContainer {
		private boolean mParcelReadyForReading;
		private Parcel mParcel;
		private BatteryStatsHistory.BatteryHistoryFragment mFragment;
		private long mMonotonicStartTime;

		BatteryHistoryParcelContainer(@NonNull Parcel parcel, long monotonicStartTime) {
		mParcel = parcel;
		mMonotonicStartTime = monotonicStartTime;
		mParcelReadyForReading = true;
		}

		BatteryHistoryParcelContainer(@NonNull BatteryHistoryFragment fragment) {
		mFragment = fragment;
		mMonotonicStartTime = fragment.monotonicTimeMs;
		mParcelReadyForReading = false;
		}

		@Nullable
		Parcel getParcel() {
		if (mParcelReadyForReading) {
		return mParcel;
		}

		Parcel parcel = Parcel.obtain();
		if (readFragmentToParcel(parcel, mFragment)) {
		parcel.readInt(); // skip buffer size
		mParcel = parcel;
		} else {
		parcel.recycle();
		}
		mParcelReadyForReading = true;
		return mParcel;
		}

		long getMonotonicStartTime() {
		return mMonotonicStartTime;
		}

		/**
		* Recycles the parcel (if appropriate). Should be called after the parcel has been
		* processed by the iterator.
		*/
		void close() {
		if (mParcel != null && mFragment != null) {
		mParcel.recycle();
		}
		// ParcelContainers are not meant to be reusable. To prevent any unintentional
		// access to the parcel after it has been recycled, clear the references to it.
		mParcel = null;
		mFragment = null;
		}
		}

		private final Parcel mHistoryBuffer;
		private final HistoryStepDetailsCalculator mStepDetailsCalculator;
		private final Clock mClock;
		@@ -447,6 +494,7 @@ public class BatteryStatsHistory {
		mWritableHistory = writableHistory;
		if (mWritableHistory != null) {
		mMutable = false;
		mHistoryBufferStartTime = mWritableHistory.mHistoryBufferStartTime;
		mHistoryMonotonicEndTime = mWritableHistory.mHistoryMonotonicEndTime;
		}

		@@ -689,95 +737,66 @@ public class BatteryStatsHistory {
		}

		/**
		* When iterating history files and history buffer, always start from the lowest numbered
		* history file, when reached the mActiveFile (highest numbered history file), do not read from
		* mActiveFile, read from history buffer instead because the buffer has more updated data.
		*
		* @return The parcel that has next record. null if finished all history files and history
		* buffer
		* Returns all chunks of accumulated history that contain items within the range between
		* `startTimeMs` (inclusive) and `endTimeMs` (exclusive)
		*/
		@Nullable
		public Parcel getNextParcel(long startTimeMs, long endTimeMs) {
		checkImmutable();

		// First iterate through all records in current parcel.
		if (mCurrentParcel != null) {
		if (mCurrentParcel.dataPosition() < mCurrentParcelEnd) {
		// There are more records in current parcel.
		return mCurrentParcel;
		} else if (mHistoryBuffer == mCurrentParcel) {
		// finished iterate through all history files and history buffer.
		return null;
		} else if (mHistoryParcels == null
		\|\| !mHistoryParcels.contains(mCurrentParcel)) {
		// current parcel is from history file.
		mCurrentParcel.recycle();
		Queue<BatteryHistoryParcelContainer> getParcelContainers(long startTimeMs, long endTimeMs) {
		if (mMutable) {
		throw new IllegalStateException("Iterating over a mutable battery history");
		}

		if (endTimeMs == MonotonicClock.UNDEFINED \|\| endTimeMs == 0) {
		endTimeMs = Long.MAX_VALUE;
		}

		Queue<BatteryHistoryParcelContainer> containers = new ArrayDeque<>();

		if (mStore != null) {
		BatteryHistoryFragment next = mStore.getNextFragment(mCurrentFragment, startTimeMs,
		endTimeMs);
		while (next != null) {
		mCurrentParcel = null;
		mCurrentParcelEnd = 0;
		final Parcel p = Parcel.obtain();
		if (readFragmentToParcel(p, next)) {
		int bufSize = p.readInt();
		int curPos = p.dataPosition();
		mCurrentParcelEnd = curPos + bufSize;
		mCurrentParcel = p;
		if (curPos < mCurrentParcelEnd) {
		mCurrentFragment = next;
		return mCurrentParcel;
		List<BatteryHistoryFragment> fragments = mStore.getFragments();
		for (int i = 0; i < fragments.size(); i++) {
		BatteryHistoryFragment fragment = fragments.get(i);
		if (fragment.monotonicTimeMs >= endTimeMs) {
		break;
		}
		} else {
		p.recycle();

		if (fragment.monotonicTimeMs >= startTimeMs && fragment != mActiveFragment) {
		containers.add(new BatteryHistoryParcelContainer(fragment));
		}
		next = mStore.getNextFragment(next, startTimeMs, endTimeMs);
		}
		}

		// mHistoryParcels is created when BatteryStatsImpl object is created from deserialization
		// of a parcel, such as Settings app or checkin file.
		if (mHistoryParcels != null) {
		while (mParcelIndex < mHistoryParcels.size()) {
		final Parcel p = mHistoryParcels.get(mParcelIndex++);
		for (int i = 0; i < mHistoryParcels.size(); i++) {
		final Parcel p = mHistoryParcels.get(i);
		if (!verifyVersion(p)) {
		continue;
		}
		// skip monotonic time field.
		p.readLong();
		// skip monotonic end time field
		p.readLong();
		// skip monotonic size field
		p.readLong();

		final int bufSize = p.readInt();
		final int curPos = p.dataPosition();
		mCurrentParcelEnd = curPos + bufSize;
		mCurrentParcel = p;
		if (curPos < mCurrentParcelEnd) {
		return mCurrentParcel;
		}
		long monotonicStartTime = p.readLong();
		if (monotonicStartTime >= endTimeMs) {
		continue;
		}

		long monotonicEndTime = p.readLong();
		if (monotonicEndTime < startTimeMs) {
		continue;
		}

		// finished iterator through history files (except the last one), now history buffer.
		if (mHistoryBuffer.dataSize() <= 0) {
		// buffer is empty.
		return null;
		// skip monotonic size field
		p.readLong();
		// skip buffer size field
		p.readInt();

		containers.add(new BatteryHistoryParcelContainer(p, monotonicStartTime));
		}
		mHistoryBuffer.setDataPosition(0);
		mCurrentParcel = mHistoryBuffer;
		mCurrentParcelEnd = mCurrentParcel.dataSize();
		return mCurrentParcel;
		}

		private void checkImmutable() {
		if (mMutable) {
		throw new IllegalStateException("Iterating over a mutable battery history");
		if (mHistoryBufferStartTime < endTimeMs) {
		mHistoryBuffer.setDataPosition(0);
		containers.add(
		new BatteryHistoryParcelContainer(mHistoryBuffer, mHistoryBufferStartTime));
		}
		return containers;
		}

		/**
		@@ -817,21 +836,6 @@ public class BatteryStatsHistory {
		return version == VERSION;
		}

		/**
		* Extracts the monotonic time, as per {@link MonotonicClock}, from the supplied battery history
		* buffer.
		*/
		public long getHistoryBufferStartTime(Parcel p) {
		int pos = p.dataPosition();
		p.setDataPosition(0);
		p.readInt(); // Skip the version field
		long monotonicTime = p.readLong();
		p.readLong(); // Skip monotonic end time field
		p.readLong(); // Skip monotonic size field
		p.setDataPosition(pos);
		return monotonicTime;
		}

		/**
		* Writes the battery history contents for persistence.
		*/

core/java/com/android/internal/os/BatteryStatsHistoryIterator.java

+49 −25

Original line number	Diff line number	Diff line
		@@ -24,6 +24,7 @@ import android.util.Slog;
		import android.util.SparseArray;

		import java.util.Iterator;
		import java.util.Queue;

		/**
		* An iterator for {@link BatteryStats.HistoryItem}'s.
		@@ -48,7 +49,10 @@ public class BatteryStatsHistoryIterator implements Iterator<BatteryStats.Histor
		private long mBaseMonotonicTime;
		private long mBaseTimeUtc;
		private int mItemIndex = 0;
		private int mMaxHistoryItems;
		private final int mMaxHistoryItems;
		private int mParcelDataPosition;

		private Queue<BatteryStatsHistory.BatteryHistoryParcelContainer> mParcelContainers;

		public BatteryStatsHistoryIterator(@NonNull BatteryStatsHistory history, long startTimeMs,
		long endTimeMs) {
		@@ -62,7 +66,11 @@ public class BatteryStatsHistoryIterator implements Iterator<BatteryStats.Histor
		@Override
		public boolean hasNext() {
		if (!mNextItemReady) {
		advance();
		if (!advance()) {
		mHistoryItem = null;
		close();
		}
		mNextItemReady = true;
		}

		return mHistoryItem != null;
		@@ -75,35 +83,48 @@ public class BatteryStatsHistoryIterator implements Iterator<BatteryStats.Histor
		@Override
		public BatteryStats.HistoryItem next() {
		if (!mNextItemReady) {
		advance();
		if (!advance()) {
		mHistoryItem = null;
		close();
		}
		}
		mNextItemReady = false;
		return mHistoryItem;
		}

		private void advance() {
		while (true) {
		if (mItemIndex > mMaxHistoryItems) {
		Slog.wtfStack(TAG, "Number of battery history items is too large: " + mItemIndex);
		break;
		private boolean advance() {
		if (mParcelContainers == null) {
		mParcelContainers = mBatteryStatsHistory.getParcelContainers(mStartTimeMs, mEndTimeMs);
		}

		Parcel p = mBatteryStatsHistory.getNextParcel(mStartTimeMs, mEndTimeMs);
		if (p == null) {
		break;
		BatteryStatsHistory.BatteryHistoryParcelContainer container;
		while ((container = mParcelContainers.peek()) != null) {
		Parcel p = container.getParcel();
		if (p == null \|\| p.dataPosition() >= p.dataSize()) {
		container.close();
		mParcelContainers.remove();
		mParcelDataPosition = 0;
		continue;
		}

		if (!mTimeInitialized) {
		mBaseMonotonicTime = mBatteryStatsHistory.getHistoryBufferStartTime(p);
		mBaseMonotonicTime = container.getMonotonicStartTime();
		mHistoryItem.time = mBaseMonotonicTime;
		mTimeInitialized = true;
		}

		try {
		readHistoryDelta(p, mHistoryItem);
		int dataPosition = p.dataPosition();
		if (dataPosition <= mParcelDataPosition) {
		Slog.wtf(TAG, "Corrupted battery history, parcel is not progressing: "
		+ dataPosition + " of " + p.dataSize());
		return false;
		}
		mParcelDataPosition = dataPosition;
		} catch (Throwable t) {
		Slog.wtf(TAG, "Corrupted battery history", t);
		break;
		return false;
		}

		if (mHistoryItem.cmd == BatteryStats.HistoryItem.CMD_CURRENT_TIME
		@@ -111,21 +132,24 @@ public class BatteryStatsHistoryIterator implements Iterator<BatteryStats.Histor
		mBaseTimeUtc = mHistoryItem.currentTime - (mHistoryItem.time - mBaseMonotonicTime);
		}

		mHistoryItem.currentTime = mBaseTimeUtc + (mHistoryItem.time - mBaseMonotonicTime);

		if (mEndTimeMs != 0 && mHistoryItem.time >= mEndTimeMs) {
		break;
		if (mHistoryItem.time < mStartTimeMs) {
		continue;
		}
		if (mHistoryItem.time >= mStartTimeMs) {
		mItemIndex++;
		mNextItemReady = true;
		return;

		if (mEndTimeMs != 0 && mEndTimeMs != MonotonicClock.UNDEFINED
		&& mHistoryItem.time >= mEndTimeMs) {
		return false;
		}

		if (mItemIndex++ > mMaxHistoryItems) {
		Slog.wtfStack(TAG, "Number of battery history items is too large: " + mItemIndex);
		return false;
		}

		mHistoryItem = null;
		mNextItemReady = true;
		close();
		mHistoryItem.currentTime = mBaseTimeUtc + (mHistoryItem.time - mBaseMonotonicTime);
		return true;
		}
		return false;
		}

		private void readHistoryDelta(Parcel src, BatteryStats.HistoryItem cur) {

services/core/java/com/android/server/power/stats/BatteryHistoryDirectory.java

+4 −38

Original line number	Diff line number	Diff line
		@@ -374,6 +374,10 @@ public class BatteryHistoryDirectory implements BatteryStatsHistory.BatteryHisto
		@SuppressWarnings("unchecked")
		@Override
		public List<BatteryHistoryFragment> getFragments() {
		if (!mLock.isHeldByCurrentThread()) {
		throw new IllegalStateException("Reading battery history without a lock");
		}

		ensureInitialized();
		return (List<BatteryHistoryFragment>)
		(List<? extends BatteryHistoryFragment>) mHistoryFiles;
		@@ -442,44 +446,6 @@ public class BatteryHistoryDirectory implements BatteryStatsHistory.BatteryHisto
		return file;
		}

		@Override
		public BatteryHistoryFragment getNextFragment(BatteryHistoryFragment current, long startTimeMs,
		long endTimeMs) {
		ensureInitialized();

		if (!mLock.isHeldByCurrentThread()) {
		throw new IllegalStateException("Iterating battery history without a lock");
		}

		int nextFileIndex = 0;
		int firstFileIndex = 0;
		// skip the last file because its data is in history buffer.
		int lastFileIndex = mHistoryFiles.size() - 2;
		for (int i = lastFileIndex; i >= 0; i--) {
		BatteryHistoryFragment fragment = mHistoryFiles.get(i);
		if (current != null && fragment.monotonicTimeMs == current.monotonicTimeMs) {
		nextFileIndex = i + 1;
		}
		if (fragment.monotonicTimeMs > endTimeMs) {
		lastFileIndex = i - 1;
		}
		if (fragment.monotonicTimeMs <= startTimeMs) {
		firstFileIndex = i;
		break;
		}
		}

		if (nextFileIndex < firstFileIndex) {
		nextFileIndex = firstFileIndex;
		}

		if (nextFileIndex <= lastFileIndex) {
		return mHistoryFiles.get(nextFileIndex);
		}

		return null;
		}

		@Override
		public boolean hasCompletedFragments() {
		ensureInitialized();

services/tests/powerstatstests/src/com/android/server/power/stats/BatteryStatsHistoryTest.java

+30 −42

Original line number	Diff line number	Diff line
		@@ -22,6 +22,7 @@ import static com.google.common.truth.Truth.assertWithMessage;
		import static org.junit.Assert.assertEquals;
		import static org.junit.Assert.assertFalse;
		import static org.junit.Assert.assertTrue;
		import static org.junit.Assert.fail;
		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.Mockito.doAnswer;
		import static org.mockito.Mockito.spy;
		@@ -330,31 +331,24 @@ public class BatteryStatsHistoryTest {
		return invocation.callRealMethod();
		}).when(mHistory).readFragmentToParcel(any(), any());

		// Prepare history for iteration
		mHistory.iterate(0, MonotonicClock.UNDEFINED);

		Parcel parcel = mHistory.getNextParcel(0, Long.MAX_VALUE);
		assertThat(parcel).isNotNull();
		int eventsRead = 0;
		BatteryStatsHistoryIterator iterator = mHistory.iterate(0, MonotonicClock.UNDEFINED);
		while (iterator.hasNext()) {
		HistoryItem item = iterator.next();
		if (item.eventCode == HistoryItem.EVENT_JOB_START) {
		eventsRead++;
		assertThat(mReadFiles).containsExactly("123.bh");
		} else if (item.eventCode == HistoryItem.EVENT_JOB_FINISH) {
		eventsRead++;
		assertThat(mReadFiles).containsExactly("123.bh", "1000.bh");
		} else if (item.eventCode == HistoryItem.EVENT_ALARM) {
		eventsRead++;
		assertThat(mReadFiles).containsExactly("123.bh", "1000.bh", "2000.bh");
		}
		}

		// Skip to the end to force reading the next parcel
		parcel.setDataPosition(parcel.dataSize());
		mReadFiles.clear();
		parcel = mHistory.getNextParcel(0, Long.MAX_VALUE);
		assertThat(parcel).isNotNull();
		assertThat(mReadFiles).containsExactly("1000.bh");

		parcel.setDataPosition(parcel.dataSize());
		mReadFiles.clear();
		parcel = mHistory.getNextParcel(0, Long.MAX_VALUE);
		assertThat(parcel).isNotNull();
		assertThat(mReadFiles).containsExactly("2000.bh");

		parcel.setDataPosition(parcel.dataSize());
		mReadFiles.clear();
		parcel = mHistory.getNextParcel(0, Long.MAX_VALUE);
		assertThat(parcel).isNull();
		assertThat(mReadFiles).isEmpty();
		assertThat(eventsRead).isEqualTo(3);
		assertThat(mReadFiles).containsExactly("123.bh", "1000.bh", "2000.bh", "3000.bh");
		}

		@Test
		@@ -372,25 +366,19 @@ public class BatteryStatsHistoryTest {
		return invocation.callRealMethod();
		}).when(mHistory).readFragmentToParcel(any(), any());

		// Prepare history for iteration
		mHistory.iterate(1000, 3000);

		Parcel parcel = mHistory.getNextParcel(1000, 3000);
		assertThat(parcel).isNotNull();
		BatteryStatsHistoryIterator iterator = mHistory.iterate(1000, 3000);
		while (iterator.hasNext()) {
		HistoryItem item = iterator.next();
		if (item.eventCode == HistoryItem.EVENT_JOB_START) {
		fail("Event outside the range");
		} else if (item.eventCode == HistoryItem.EVENT_JOB_FINISH) {
		assertThat(mReadFiles).containsExactly("1000.bh");
		} else if (item.eventCode == HistoryItem.EVENT_ALARM) {
		fail("Event outside the range");
		}
		}

		// Skip to the end to force reading the next parcel
		parcel.setDataPosition(parcel.dataSize());
		mReadFiles.clear();
		parcel = mHistory.getNextParcel(1000, 3000);
		assertThat(parcel).isNotNull();
		assertThat(mReadFiles).containsExactly("2000.bh");

		parcel.setDataPosition(parcel.dataSize());
		mReadFiles.clear();
		parcel = mHistory.getNextParcel(1000, 3000);
		assertThat(parcel).isNull();
		assertThat(mReadFiles).isEmpty();
		assertThat(mReadFiles).containsExactly("1000.bh", "2000.bh");
		}

		private void prepareMultiFileHistory() {