From 2b92fa5323e1c0fbc6edcc19a5711ff4db8326cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 27 Jan 2020 17:36:56 -0800 Subject: [PATCH] Fix incorrect interpolation of active bucket for partial requests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just because the bucket is active (ie. overlaps with 'now') doesn't mean the request necessarily covers the entire bucket. For example a bucketDuration=5 with a bucket from curStart=1 to curEnd=6 with now=4 and requests: - from start=0 to end=3 --> overlap=[1,3] --> bucketSpan=2 - from start=2 to end=3 --> overlap=[2,3] --> bucketSpan=1 - from start=2 to end=5 --> overlap=[2,4] --> bucketSpan=2 - from start=2 to end=7 --> overlap=[2,4] --> bucketSpan=2 and the previously correctly handled cases: - from start=0 to end=5 --> overlap=[1,4] --> bucketSpan=3 - from start=0 to end=7 --> overlap=[1,4] --> bucketSpan=3 We fix this by bounding the end of buckets to now, this means that bucketDuration is no longer a constant, but it simplifies all the rest of the logic. Test: builds, atest Bug: 143338233 Signed-off-by: Maciej Żenczykowski Change-Id: I642b57f7b8486071ad7808fd9b901859923b6d25 --- .../java/android/net/NetworkStatsHistory.java | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/core/java/android/net/NetworkStatsHistory.java b/core/java/android/net/NetworkStatsHistory.java index 7b799e2624..c82c288222 100644 --- a/core/java/android/net/NetworkStatsHistory.java +++ b/core/java/android/net/NetworkStatsHistory.java @@ -548,32 +548,32 @@ public class NetworkStatsHistory implements Parcelable { final int startIndex = getIndexAfter(end); for (int i = startIndex; i >= 0; i--) { final long curStart = bucketStart[i]; - final long curEnd = curStart + bucketDuration; + long curEnd = curStart + bucketDuration; // bucket is older than request; we're finished if (curEnd <= start) break; // bucket is newer than request; keep looking if (curStart >= end) continue; - // include full value for active buckets, otherwise only fractional - final boolean activeBucket = curStart < now && curEnd > now; - final long overlap; - if (activeBucket) { - overlap = bucketDuration; - } else { - final long overlapEnd = curEnd < end ? curEnd : end; - final long overlapStart = curStart > start ? curStart : start; - overlap = overlapEnd - overlapStart; - } + // the active bucket is shorter then a normal completed bucket + if (curEnd > now) curEnd = now; + // usually this is simply bucketDuration + final long bucketSpan = curEnd - curStart; + // prevent division by zero + if (bucketSpan <= 0) continue; + + final long overlapEnd = curEnd < end ? curEnd : end; + final long overlapStart = curStart > start ? curStart : start; + final long overlap = overlapEnd - overlapStart; if (overlap <= 0) continue; // integer math each time is faster than floating point - if (activeTime != null) entry.activeTime += activeTime[i] * overlap / bucketDuration; - if (rxBytes != null) entry.rxBytes += rxBytes[i] * overlap / bucketDuration; - if (rxPackets != null) entry.rxPackets += rxPackets[i] * overlap / bucketDuration; - if (txBytes != null) entry.txBytes += txBytes[i] * overlap / bucketDuration; - if (txPackets != null) entry.txPackets += txPackets[i] * overlap / bucketDuration; - if (operations != null) entry.operations += operations[i] * overlap / bucketDuration; + if (activeTime != null) entry.activeTime += activeTime[i] * overlap / bucketSpan; + if (rxBytes != null) entry.rxBytes += rxBytes[i] * overlap / bucketSpan; + if (rxPackets != null) entry.rxPackets += rxPackets[i] * overlap / bucketSpan; + if (txBytes != null) entry.txBytes += txBytes[i] * overlap / bucketSpan; + if (txPackets != null) entry.txPackets += txPackets[i] * overlap / bucketSpan; + if (operations != null) entry.operations += operations[i] * overlap / bucketSpan; } return entry; }