From 4cee725b1fd3958d850fc83214797f76d5f6b468 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Fri, 19 Mar 2010 14:50:40 -0700 Subject: [PATCH] Use atomic++ rather than lock/++/unlock in the input dispatch code path Decouples the input dispatch thread from the battery-stats object lock regime, to avoid the possibility of ever blocking the input dispatch thread on its behalf. The stats object is widely used and can sometimes be locked for a very long time (on the order of seconds) during certain extensive dump operations. This change does not alter the data format of the battery stats' externalized representations. Fixes bug #2530346 Change-Id: Iee288be3bf4936641b532dceecb8f6de8f552bf0 --- .../android/internal/os/BatteryStatsImpl.java | 43 +++++++++++-------- .../server/am/BatteryStatsService.java | 4 +- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 46769ce69177a..24275ec4ac31d 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -46,6 +46,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; /** * All information we are collecting about things that can happen that impact @@ -230,14 +231,15 @@ public final class BatteryStatsImpl extends BatteryStats { * State for keeping track of counting information. */ public static class Counter extends BatteryStats.Counter implements Unpluggable { - int mCount; + final AtomicInteger mCount = new AtomicInteger(); int mLoadedCount; int mLastCount; int mUnpluggedCount; int mPluggedCount; Counter(ArrayList unpluggables, Parcel in) { - mPluggedCount = mCount = in.readInt(); + mPluggedCount = in.readInt(); + mCount.set(mPluggedCount); mLoadedCount = in.readInt(); mLastCount = in.readInt(); mUnpluggedCount = in.readInt(); @@ -249,18 +251,19 @@ public final class BatteryStatsImpl extends BatteryStats { } public void writeToParcel(Parcel out) { - out.writeInt(mCount); + out.writeInt(mCount.get()); out.writeInt(mLoadedCount); out.writeInt(mLastCount); out.writeInt(mUnpluggedCount); } public void unplug(long batteryUptime, long batteryRealtime) { - mUnpluggedCount = mCount = mPluggedCount; + mUnpluggedCount = mPluggedCount; + mCount.set(mPluggedCount); } public void plug(long batteryUptime, long batteryRealtime) { - mPluggedCount = mCount; + mPluggedCount = mCount.get(); } /** @@ -285,7 +288,7 @@ public final class BatteryStatsImpl extends BatteryStats { if (which == STATS_LAST) { val = mLastCount; } else { - val = mCount; + val = mCount.get(); if (which == STATS_UNPLUGGED) { val -= mUnpluggedCount; } else if (which != STATS_TOTAL) { @@ -297,25 +300,27 @@ public final class BatteryStatsImpl extends BatteryStats { } public void logState(Printer pw, String prefix) { - pw.println(prefix + "mCount=" + mCount + pw.println(prefix + "mCount=" + mCount.get() + " mLoadedCount=" + mLoadedCount + " mLastCount=" + mLastCount + " mUnpluggedCount=" + mUnpluggedCount + " mPluggedCount=" + mPluggedCount); } - void stepLocked() { - mCount++; + void stepAtomic() { + mCount.incrementAndGet(); } void writeSummaryFromParcelLocked(Parcel out) { - out.writeInt(mCount); - out.writeInt(mCount - mLoadedCount); + int count = mCount.get(); + out.writeInt(count); + out.writeInt(count - mLoadedCount); } void readSummaryFromParcelLocked(Parcel in) { - mCount = mLoadedCount = in.readInt(); + mLoadedCount = in.readInt(); + mCount.set(mLoadedCount); mLastCount = in.readInt(); - mUnpluggedCount = mPluggedCount = mCount; + mUnpluggedCount = mPluggedCount = mLoadedCount; } } @@ -329,8 +334,8 @@ public final class BatteryStatsImpl extends BatteryStats { super(unpluggables); } - public void addCountLocked(long count) { - mCount += count; + public void addCountAtomic(long count) { + mCount.addAndGet((int)count); } } @@ -1124,8 +1129,8 @@ public final class BatteryStatsImpl extends BatteryStats { } } - public void noteInputEventLocked() { - mInputEventCounter.stepLocked(); + public void noteInputEventAtomic() { + mInputEventCounter.stepAtomic(); } public void noteUserActivityLocked(int uid, int event) { @@ -1680,7 +1685,7 @@ public final class BatteryStatsImpl extends BatteryStats { } if (type < 0) type = 0; else if (type >= NUM_USER_ACTIVITY_TYPES) type = NUM_USER_ACTIVITY_TYPES-1; - mUserActivityCounters[type].stepLocked(); + mUserActivityCounters[type].stepAtomic(); } @Override @@ -2172,7 +2177,7 @@ public final class BatteryStatsImpl extends BatteryStats { /* Called by ActivityManagerService when CPU times are updated. */ public void addSpeedStepTimes(long[] values) { for (int i = 0; i < mSpeedBins.length && i < values.length; i++) { - mSpeedBins[i].addCountLocked(values[i]); + mSpeedBins[i].addCountAtomic(values[i]); } } diff --git a/services/java/com/android/server/am/BatteryStatsService.java b/services/java/com/android/server/am/BatteryStatsService.java index cdf4e95b39ccc..33bbc1339f254 100644 --- a/services/java/com/android/server/am/BatteryStatsService.java +++ b/services/java/com/android/server/am/BatteryStatsService.java @@ -158,9 +158,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub { public void noteInputEvent() { enforceCallingPermission(); - synchronized (mStats) { - mStats.noteInputEventLocked(); - } + mStats.noteInputEventAtomic(); } public void noteUserActivity(int uid, int event) {