From f44fe9c4b694bf993b440aa721ac8f3e747ab5c7 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 --- .../stats/pull/StatsPullAtomService.java | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 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 539b4138cc18f..0d4360058788b 100644 --- a/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java +++ b/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java @@ -1612,9 +1612,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. @@ -1622,26 +1619,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 { // 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]; } });