Fixing issue in current and max duration calculations.

It looks like one operation was done out of order and some of the times used in
the calculations were leading to incorrect results.

BUG: 31023263
Test: bit FrameworksCoreTests:com.android.internal.os.BatteryStatsDurationTimerTest

Change-Id: I417cc28c5a55748067b6c7f682a66fe3dbc09f09
(cherry picked from commit 47db5a8bf7)
This commit is contained in:
Kweku Adams
2016-12-09 19:04:50 -08:00
parent 7af83d0e5a
commit c6bd4243a4
3 changed files with 26 additions and 28 deletions

View File

@@ -182,7 +182,7 @@ public abstract class BatteryStats implements Parcelable {
* New in version 19: * New in version 19:
* - Wakelock data (wl) gets current and max times. * - Wakelock data (wl) gets current and max times.
*/ */
static final String CHECKIN_VERSION = "19"; static final String CHECKIN_VERSION = "20";
/** /**
* Old version, we hit 9 and ran out of room, need to remove. * Old version, we hit 9 and ran out of room, need to remove.

View File

@@ -108,7 +108,7 @@ public class BatteryStatsImpl extends BatteryStats {
private static final int MAGIC = 0xBA757475; // 'BATSTATS' private static final int MAGIC = 0xBA757475; // 'BATSTATS'
// Current on-disk Parcel version // Current on-disk Parcel version
private static final int VERSION = 150 + (USE_OLD_HISTORY ? 1000 : 0); private static final int VERSION = 151 + (USE_OLD_HISTORY ? 1000 : 0);
// Maximum number of items we will record in the history. // Maximum number of items we will record in the history.
private static final int MAX_HISTORY_ITEMS = 2000; private static final int MAX_HISTORY_ITEMS = 2000;
@@ -1593,7 +1593,7 @@ public class BatteryStatsImpl extends BatteryStats {
@Override @Override
public void writeToParcel(Parcel out, long elapsedRealtimeUs) { public void writeToParcel(Parcel out, long elapsedRealtimeUs) {
super.writeToParcel(out, elapsedRealtimeUs); super.writeToParcel(out, elapsedRealtimeUs);
out.writeLong(mMaxDurationMs); out.writeLong(getMaxDurationMsLocked(elapsedRealtimeUs / 1000));
} }
/** /**
@@ -1606,7 +1606,7 @@ public class BatteryStatsImpl extends BatteryStats {
@Override @Override
public void writeSummaryFromParcelLocked(Parcel out, long elapsedRealtimeUs) { public void writeSummaryFromParcelLocked(Parcel out, long elapsedRealtimeUs) {
super.writeSummaryFromParcelLocked(out, elapsedRealtimeUs); super.writeSummaryFromParcelLocked(out, elapsedRealtimeUs);
out.writeLong(mMaxDurationMs); out.writeLong(getMaxDurationMsLocked(elapsedRealtimeUs / 1000));
} }
/** /**
@@ -1630,7 +1630,7 @@ public class BatteryStatsImpl extends BatteryStats {
public void onTimeStarted(long elapsedRealtimeUs, long baseUptime, long baseRealtime) { public void onTimeStarted(long elapsedRealtimeUs, long baseUptime, long baseRealtime) {
super.onTimeStarted(elapsedRealtimeUs, baseUptime, baseRealtime); super.onTimeStarted(elapsedRealtimeUs, baseUptime, baseRealtime);
if (mNesting > 0) { if (mNesting > 0) {
mStartTimeMs = mTimeBase.getRealtime(mClocks.elapsedRealtime()*1000) / 1000; mStartTimeMs = baseRealtime / 1000;
} }
} }
@@ -1640,10 +1640,11 @@ public class BatteryStatsImpl extends BatteryStats {
* If the timer is running, add the duration into mCurrentDurationMs. * If the timer is running, add the duration into mCurrentDurationMs.
*/ */
@Override @Override
public void onTimeStopped(long elapsedRealtimeUs, long baseUptime, long baseRealtime) { public void onTimeStopped(long elapsedRealtimeUs, long baseUptime, long baseRealtimeUs) {
super.onTimeStopped(elapsedRealtimeUs, baseUptime, baseRealtime); super.onTimeStopped(elapsedRealtimeUs, baseUptime, baseRealtimeUs);
if (mNesting > 0) { if (mNesting > 0) {
mCurrentDurationMs += (elapsedRealtimeUs / 1000) - mStartTimeMs; // baseRealtimeUs has already been converted to the timebase's realtime.
mCurrentDurationMs += (baseRealtimeUs / 1000) - mStartTimeMs;
} }
mStartTimeMs = -1; mStartTimeMs = -1;
} }
@@ -1658,7 +1659,7 @@ public class BatteryStatsImpl extends BatteryStats {
super.startRunningLocked(elapsedRealtimeMs); super.startRunningLocked(elapsedRealtimeMs);
if (mNesting == 1 && mTimeBase.isRunning()) { if (mNesting == 1 && mTimeBase.isRunning()) {
// Just started // Just started
mStartTimeMs = mTimeBase.getRealtime(mClocks.elapsedRealtime()*1000) / 1000; mStartTimeMs = mTimeBase.getRealtime(elapsedRealtimeMs * 1000) / 1000;
} }
} }
@@ -1670,8 +1671,7 @@ public class BatteryStatsImpl extends BatteryStats {
*/ */
@Override @Override
public void stopRunningLocked(long elapsedRealtimeMs) { public void stopRunningLocked(long elapsedRealtimeMs) {
super.stopRunningLocked(elapsedRealtimeMs); if (mNesting == 1) {
if (mNesting == 0) {
final long durationMs = getCurrentDurationMsLocked(elapsedRealtimeMs); final long durationMs = getCurrentDurationMsLocked(elapsedRealtimeMs);
if (durationMs > mMaxDurationMs) { if (durationMs > mMaxDurationMs) {
mMaxDurationMs = durationMs; mMaxDurationMs = durationMs;
@@ -1679,6 +1679,9 @@ public class BatteryStatsImpl extends BatteryStats {
mStartTimeMs = -1; mStartTimeMs = -1;
mCurrentDurationMs = 0; mCurrentDurationMs = 0;
} }
// super method decrements mNesting, which getCurrentDurationMsLocked relies on,
// so call super.stopRunningLocked after calling getCurrentDurationMsLocked.
super.stopRunningLocked(elapsedRealtimeMs);
} }
@Override @Override
@@ -1720,11 +1723,9 @@ public class BatteryStatsImpl extends BatteryStats {
@Override @Override
public long getCurrentDurationMsLocked(long elapsedRealtimeMs) { public long getCurrentDurationMsLocked(long elapsedRealtimeMs) {
long durationMs = mCurrentDurationMs; long durationMs = mCurrentDurationMs;
if (mNesting > 0) { if (mNesting > 0 && mTimeBase.isRunning()) {
if (mTimeBase.isRunning()) { durationMs += (mTimeBase.getRealtime(elapsedRealtimeMs*1000)/1000)
durationMs += (mTimeBase.getRealtime(elapsedRealtimeMs*1000)/1000) - mStartTimeMs;
- mStartTimeMs;
}
} }
return durationMs; return durationMs;
} }

View File

@@ -18,12 +18,9 @@ package com.android.internal.os;
import android.os.BatteryStats; import android.os.BatteryStats;
import android.os.Parcel; import android.os.Parcel;
import android.support.test.filters.SmallTest; import android.support.test.filters.SmallTest;
import android.util.Log;
import junit.framework.TestCase; import junit.framework.TestCase;
import org.mockito.Mockito;
/** /**
* Test BatteryStatsImpl.DurationTimer. * Test BatteryStatsImpl.DurationTimer.
* *
@@ -82,15 +79,15 @@ public class BatteryStatsDurationTimerTest extends TestCase {
// Stop the TimeBase. The values should be frozen. // Stop the TimeBase. The values should be frozen.
timeBase.setRunning(false, /* uptimeUs */ 10, /* realtimeUs */ 55000*1000); timeBase.setRunning(false, /* uptimeUs */ 10, /* realtimeUs */ 55000*1000);
assertTrue(timer.isRunningLocked()); assertTrue(timer.isRunningLocked());
assertEquals(28100, timer.getCurrentDurationMsLocked(110100)); // Why 28100 and not 28000? assertEquals(28000, timer.getCurrentDurationMsLocked(110100));
assertEquals(28100, timer.getMaxDurationMsLocked(110101)); assertEquals(28000, timer.getMaxDurationMsLocked(110101));
// Start the TimeBase. The values should be the old value plus the delta // Start the TimeBase. The values should be the old value plus the delta
// between when the timer restarted and the current time // between when the timer restarted and the current time
timeBase.setRunning(true, /* uptimeUs */ 10, /* realtimeUs */ 220100*1000); timeBase.setRunning(true, /* uptimeUs */ 10, /* realtimeUs */ 220100*1000);
assertTrue(timer.isRunningLocked()); assertTrue(timer.isRunningLocked());
assertEquals(28300, timer.getCurrentDurationMsLocked(220300)); // extra 100 from above?? assertEquals(28200, timer.getCurrentDurationMsLocked(220300));
assertEquals(28301, timer.getMaxDurationMsLocked(220301)); assertEquals(28201, timer.getMaxDurationMsLocked(220301));
} }
@SmallTest @SmallTest