Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2c11a679 authored by Dmitri Plotnikov's avatar Dmitri Plotnikov
Browse files

Address concurrency issues with HistoryStepDetails recording

Bug: 399764136
Test: atest PowerStatsTests; atest PowerStatsTestsRavenwood
Flag: EXEMPT bugfix

Change-Id: I757e92ef0bac4c4b5a35d42c748392002878498b
parent 65b2d14c
Loading
Loading
Loading
Loading
+14 −3
Original line number Diff line number Diff line
@@ -1780,7 +1780,7 @@ public abstract class BatteryStats {
            out.writeInt(statIrqTime);
            out.writeInt(statSoftIrqTime);
            out.writeInt(statIdlTime);
            out.writeString(statSubsystemPowerState);
            out.writeString8(statSubsystemPowerState);
        }

        public void readFromParcel(Parcel in) {
@@ -1801,7 +1801,15 @@ public abstract class BatteryStats {
            statIrqTime = in.readInt();
            statSoftIrqTime = in.readInt();
            statIdlTime = in.readInt();
            statSubsystemPowerState = in.readString();
            statSubsystemPowerState = in.readString8();
        }


        public boolean isEmpty() {
            return userTime == 0 && systemTime == 0 && appCpuUid1 == Process.INVALID_UID
                    && appCpuUid2 == Process.INVALID_UID && appCpuUid3 == Process.INVALID_UID
                    && statSystemTime == 0 && statIOWaitTime == 0 && statIrqTime == 0
                    && statSoftIrqTime == 0 && statIdlTime == 0 && statSubsystemPowerState == null;
        }
    }

@@ -2238,6 +2246,7 @@ public abstract class BatteryStats {
            tagsFirstOccurrence = false;
            powerStats = null;
            processStateChange = null;
            stepDetails = null;
        }

        @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P)
@@ -2289,6 +2298,7 @@ public abstract class BatteryStats {
            currentTime = o.currentTime;
            powerStats = o.powerStats;
            processStateChange = o.processStateChange;
            stepDetails = o.stepDetails;
        }

        public boolean sameNonEvent(HistoryItem o) {
@@ -7318,7 +7328,8 @@ public abstract class BatteryStats {
                        }

                        item.append(", SubsystemPowerState ");
                        item.append(rec.stepDetails.statSubsystemPowerState);
                        item.append(rec.stepDetails.statSubsystemPowerState != null
                                ? rec.stepDetails.statSubsystemPowerState : "Empty");
                        item.append("\n");
                    } else {
                        item.append(BATTERY_STATS_CHECKIN_VERSION); item.append(',');
+26 −66
Original line number Diff line number Diff line
@@ -79,7 +79,7 @@ public class BatteryStatsHistory {
    private static final String TAG = "BatteryStatsHistory";

    // Current on-disk Parcel version. Must be updated when the format of the parcelable changes
    private static final int VERSION = 213;
    private static final int VERSION = 214;

    // Part of initial delta int that specifies the time delta.
    static final int DELTA_TIME_MASK = 0x7ffff;
@@ -316,7 +316,6 @@ public class BatteryStatsHistory {
    }

    private final Parcel mHistoryBuffer;
    private final HistoryStepDetailsCalculator mStepDetailsCalculator;
    private final Clock mClock;

    private int mMaxHistoryBufferSize;
@@ -337,25 +336,6 @@ public class BatteryStatsHistory {
     */
    private List<Parcel> mHistoryParcels = null;

    /**
     * When iterating history files, the current file index.
     */
    private BatteryHistoryFragment mCurrentFragment;

    /**
     * When iterating history files, the current file parcel.
     */
    private Parcel mCurrentParcel;
    /**
     * When iterating history file, the current parcel's Parcel.dataSize().
     */
    private int mCurrentParcelEnd;
    /**
     * Used when BatteryStatsImpl object is created from deserialization of a parcel,
     * such as Settings app or checkin file, to iterate over history parcels.
     */
    private int mParcelIndex = 0;

    private final ReentrantLock mWriteLock = new ReentrantLock();

    private final HistoryItem mHistoryCur = new HistoryItem();
@@ -384,27 +364,10 @@ public class BatteryStatsHistory {
    // Monotonically increasing size of written history
    private long mMonotonicHistorySize;
    private final ArraySet<PowerStats.Descriptor> mWrittenPowerStatsDescriptors = new ArraySet<>();
    private byte mLastHistoryStepLevel = 0;
    private boolean mMutable = true;
    private int mIteratorCookie;
    private final BatteryStatsHistory mWritableHistory;

    /**
     * A delegate responsible for computing additional details for a step in battery history.
     */
    public interface HistoryStepDetailsCalculator {
        /**
         * Returns additional details for the current history step or null.
         */
        @Nullable
        HistoryStepDetails getHistoryStepDetails();

        /**
         * Resets the calculator to get ready for a new battery session
         */
        void clear();
    }

    /**
     * A delegate for android.os.Trace to allow testing static calls. Due to
     * limitations in Android Tracing (b/153319140), the delegate also records
@@ -472,21 +435,17 @@ public class BatteryStatsHistory {
     * @param maxHistoryBufferSize the most amount of RAM to used for buffering of history steps
     */
    public BatteryStatsHistory(Parcel historyBuffer, int maxHistoryBufferSize,
            @Nullable BatteryHistoryStore store, HistoryStepDetailsCalculator stepDetailsCalculator,
            Clock clock, MonotonicClock monotonicClock, TraceDelegate tracer,
            EventLogger eventLogger) {
        this(historyBuffer, maxHistoryBufferSize, store,
                stepDetailsCalculator,
                clock, monotonicClock, tracer, eventLogger, null);
            @Nullable BatteryHistoryStore store, Clock clock, MonotonicClock monotonicClock,
            TraceDelegate tracer, EventLogger eventLogger) {
        this(historyBuffer, maxHistoryBufferSize, store, clock, monotonicClock, tracer, eventLogger,
                null);
    }

    private BatteryStatsHistory(@Nullable Parcel historyBuffer, int maxHistoryBufferSize,
            @Nullable BatteryHistoryStore store,
            @NonNull HistoryStepDetailsCalculator stepDetailsCalculator, @NonNull Clock clock,
            @Nullable BatteryHistoryStore store, @NonNull Clock clock,
            @NonNull MonotonicClock monotonicClock, @NonNull TraceDelegate tracer,
            @NonNull EventLogger eventLogger, @Nullable BatteryStatsHistory writableHistory) {
        mMaxHistoryBufferSize = maxHistoryBufferSize;
        mStepDetailsCalculator = stepDetailsCalculator;
        mTracer = tracer;
        mClock = clock;
        mMonotonicClock = monotonicClock;
@@ -527,7 +486,6 @@ public class BatteryStatsHistory {
        mClock = Clock.SYSTEM_CLOCK;
        mTracer = null;
        mStore = null;
        mStepDetailsCalculator = null;
        mEventLogger = new EventLogger();
        mWritableHistory = null;
        mMutable = false;
@@ -556,9 +514,6 @@ public class BatteryStatsHistory {
        mNextHistoryTagIdx = 0;
        mNumHistoryTagChars = 0;
        mHistoryBufferLastPos = -1;
        if (mStepDetailsCalculator != null) {
            mStepDetailsCalculator.clear();
        }
    }

    /**
@@ -596,7 +551,7 @@ public class BatteryStatsHistory {
                Parcel historyBufferCopy = Parcel.obtain();
                historyBufferCopy.appendFrom(mHistoryBuffer, 0, mHistoryBuffer.dataSize());

                return new BatteryStatsHistory(historyBufferCopy, 0, mStore, null,
                return new BatteryStatsHistory(historyBufferCopy, 0, mStore,
                        null, null, null, mEventLogger, this);
            }
        } finally {
@@ -712,10 +667,6 @@ public class BatteryStatsHistory {
        if (mStore != null) {
            mStore.lock();
        }
        mCurrentFragment = null;
        mCurrentParcel = null;
        mCurrentParcelEnd = 0;
        mParcelIndex = 0;
        BatteryStatsHistoryIterator iterator = new BatteryStatsHistoryIterator(
                this, startTimeMs, endTimeMs);
        mIteratorCookie = System.identityHashCode(iterator);
@@ -1613,6 +1564,21 @@ public class BatteryStatsHistory {
        return (bits & ~mask) | shiftedValue;
    }

    /**
     * Records an update containing HistoryStepDetails, except if the details are empty.
     */
    public void recordHistoryStepDetails(HistoryStepDetails details, long elapsedRealtimeMs,
            long uptimeMs) {
        if (details.isEmpty()) {
            return;
        }
        synchronized (this) {
            mHistoryCur.stepDetails = details;
            writeHistoryItem(elapsedRealtimeMs, uptimeMs);
            mHistoryCur.stepDetails = null;
        }
    }

    /**
     * Writes the current history item to history.
     */
@@ -1632,8 +1598,8 @@ public class BatteryStatsHistory {
                    mHistoryAddTmp.processStateChange = null;
                    mHistoryAddTmp.eventCode = HistoryItem.EVENT_NONE;
                    mHistoryAddTmp.states &= ~HistoryItem.STATE_CPU_RUNNING_FLAG;
                    mHistoryAddTmp.stepDetails = null;
                    writeHistoryItem(wakeElapsedTimeMs, uptimeMs, mHistoryAddTmp);

                }
            }
            mHistoryCur.states |= HistoryItem.STATE_CPU_RUNNING_FLAG;
@@ -1952,15 +1918,8 @@ public class BatteryStatsHistory {
        }
        int firstToken = deltaTimeToken | (cur.states & BatteryStatsHistory.DELTA_STATE_MASK);

        if (cur.batteryLevel < mLastHistoryStepLevel || mLastHistoryStepLevel == 0) {
            cur.stepDetails = mStepDetailsCalculator.getHistoryStepDetails();
        if (cur.stepDetails != null) {
            batteryLevelInt |= BatteryStatsHistory.BATTERY_LEVEL_DETAILS_FLAG;
                mLastHistoryStepLevel = cur.batteryLevel;
            }
        } else {
            cur.stepDetails = null;
            mLastHistoryStepLevel = cur.batteryLevel;
        }

        final boolean batteryLevelIntChanged = batteryLevelInt != 0;
@@ -2055,6 +2014,7 @@ public class BatteryStatsHistory {
                        + Integer.toHexString(cur.states2));
            }
        }
        cur.tagsFirstOccurrence = false;
        if (cur.wakelockTag != null || cur.wakeReasonTag != null) {
            int wakeLockIndex;
            int wakeReasonIndex;
+1 −1
Original line number Diff line number Diff line
@@ -2264,9 +2264,9 @@ public class AppProfiler {
                        final int idleTime = mProcessCpuTracker.getLastIdleTime();
                        bstats.addCpuStatsLocked(totalUTime, totalSTime, userTime,
                                systemTime, iowaitTime, irqTime, softIrqTime, idleTime);
                    }
                        bstats.finishAddingCpuStatsLocked();
                    }
                }

                if (mLastWriteTime < (now - BATTERY_STATS_TIME)) {
                    mLastWriteTime = now;
+0 −50
Original line number Diff line number Diff line
@@ -211,7 +211,6 @@ public final class BatteryStatsService extends IBatteryStats.Stub
    private static final int POWER_STATS_QUERY_TIMEOUT_MILLIS = 2000;
    private static final String DEVICE_CONFIG_NAMESPACE = "backstage_power";
    private static final String MIN_CONSUMED_POWER_THRESHOLD_KEY = "min_consumed_power_threshold";
    private static final String EMPTY = "Empty";

    private final HandlerThread mHandlerThread;
    private final Handler mHandler;
@@ -336,55 +335,6 @@ public final class BatteryStatsService extends IBatteryStats.Stub
        }
    }

    @Override
    public String getSubsystemLowPowerStats() {
        synchronized (mPowerStatsLock) {
            if (mPowerStatsInternal == null || mEntityNames.isEmpty() || mStateNames.isEmpty()) {
                return EMPTY;
            }
        }

        final StateResidencyResult[] results;
        try {
            results = mPowerStatsInternal.getStateResidencyAsync(new int[0])
                    .get(POWER_STATS_QUERY_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS);
        } catch (Exception e) {
            Slog.e(TAG, "Failed to getStateResidencyAsync", e);
            return EMPTY;
        }

        if (results == null || results.length == 0) return EMPTY;

        int charsLeft = MAX_LOW_POWER_STATS_SIZE;
        StringBuilder builder = new StringBuilder("SubsystemPowerState");
        for (int i = 0; i < results.length; i++) {
            final StateResidencyResult result = results[i];
            StringBuilder subsystemBuilder = new StringBuilder();
            subsystemBuilder.append(" subsystem_" + i);
            subsystemBuilder.append(" name=" + mEntityNames.get(result.id));

            for (int j = 0; j < result.stateResidencyData.length; j++) {
                final StateResidency stateResidency = result.stateResidencyData[j];
                subsystemBuilder.append(" state_" + j);
                subsystemBuilder.append(" name=" + mStateNames.get(result.id).get(
                        stateResidency.id));
                subsystemBuilder.append(" time=" + stateResidency.totalTimeInStateMs);
                subsystemBuilder.append(" count=" + stateResidency.totalStateEntryCount);
                subsystemBuilder.append(" last entry=" + stateResidency.lastEntryTimestampMs);
            }

            if (subsystemBuilder.length() <= charsLeft) {
                charsLeft -= subsystemBuilder.length();
                builder.append(subsystemBuilder);
            } else {
                Slog.e(TAG, "getSubsystemLowPowerStats: buffer not enough");
                break;
            }
        }

        return builder.toString();
    }

    private ConnectivityManager.NetworkCallback mNetworkCallback =
            new ConnectivityManager.NetworkCallback() {
        @Override
+0 −9
Original line number Diff line number Diff line
@@ -272,15 +272,6 @@ public class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStat
        mHandler.removeMessages(SYNC_WAKELOCK_CHANGE);
    }

    @Override
    public void scheduleSyncDueToBatteryLevelChange(long delayMillis) {
        synchronized (BatteryExternalStatsWorker.this) {
            scheduleDelayedSyncLocked(SYNC_BATTERY_LEVEL_CHANGE,
                    () -> scheduleSync("battery-level", UPDATE_ALL),
                    delayMillis);
        }
    }

    @GuardedBy("this")
    private void cancelSyncDueToBatteryLevelChangeLocked() {
        mHandler.removeMessages(SYNC_BATTERY_LEVEL_CHANGE);
Loading