From 1cff69d2c9b6f5c39830c6ce6e079217a9f22a7c Mon Sep 17 00:00:00 2001 From: Rafal Slawik Date: Thu, 11 Feb 2021 10:54:24 +0000 Subject: [PATCH] Do not keep the input array with CPU times CpuTimePerUidFreq puller kept the input array with CPU times to avoid copies, but that array is owned by the reader and is mutated for each callback invocation. This corrupted the aggregated data. Bug: 157535126 Test: cmd stats pull-source 10010 Test: atest CpuStatsTests Change-Id: Icda6a71f558e582c5d5b497e2cca1d84deed8b99 Merged-In: Icda6a71f558e582c5d5b497e2cca1d84deed8b99 --- .../stats/pull/StatsPullAtomService.java | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java b/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java index 96b53f0baa354..eb6b734807b09 100644 --- a/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java +++ b/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java @@ -1565,9 +1565,6 @@ public class StatsPullAtomService extends SystemService { // Aggregate times for the same uids. SparseArray aggregated = new SparseArray<>(); mCpuUidFreqTimeReader.readAbsolute((uid, cpuFreqTimeMs) -> { - // For uids known to be aggregated from many entries allow mutating in place to avoid - // many copies. Otherwise, copy before aggregating. - boolean mutateInPlace = false; if (UserHandle.isIsolated(uid)) { // Skip individual isolated uids because they are recycled and quickly removed from // the underlying data source. @@ -1575,26 +1572,18 @@ public class StatsPullAtomService extends SystemService { } else if (UserHandle.isSharedAppGid(uid)) { // All shared app gids are accounted together. uid = LAST_SHARED_APPLICATION_GID; - mutateInPlace = true; - } else if (UserHandle.isApp(uid)) { - // Apps are accounted under their app id. + } else { + // Everything else is accounted under their base uid. uid = UserHandle.getAppId(uid); } long[] aggCpuFreqTimeMs = aggregated.get(uid); - if (aggCpuFreqTimeMs != null) { - if (!mutateInPlace) { - aggCpuFreqTimeMs = Arrays.copyOf(aggCpuFreqTimeMs, cpuFreqTimeMs.length); - aggregated.put(uid, aggCpuFreqTimeMs); - } - for (int freqIndex = 0; freqIndex < cpuFreqTimeMs.length; ++freqIndex) { - aggCpuFreqTimeMs[freqIndex] += cpuFreqTimeMs[freqIndex]; - } - } else { - if (mutateInPlace) { - cpuFreqTimeMs = Arrays.copyOf(cpuFreqTimeMs, cpuFreqTimeMs.length); - } - aggregated.put(uid, cpuFreqTimeMs); + if (aggCpuFreqTimeMs == null) { + aggCpuFreqTimeMs = new long[cpuFreqTimeMs.length]; + aggregated.put(uid, aggCpuFreqTimeMs); + } + for (int freqIndex = 0; freqIndex < cpuFreqTimeMs.length; ++freqIndex) { + aggCpuFreqTimeMs[freqIndex] += cpuFreqTimeMs[freqIndex]; } });