From d7af8599aa351046b613592a34c7933a1671a597 Mon Sep 17 00:00:00 2001 From: Siim Sammul Date: Wed, 28 Apr 2021 16:31:01 +0100 Subject: [PATCH] Add sharding to Binder Latency collection and make it configurable. The hash code of the binder class will be used to select which apis will be collected for based on the configured modulo. Test: unit tests Bug: 180584913 Change-Id: I7544135a77cf510fa9e542d98262a7211760935c --- .../android/internal/os/BinderCallsStats.java | 7 +- .../internal/os/BinderLatencyObserver.java | 46 +++++++++++-- .../os/BinderLatencyObserverTest.java | 68 +++++++++++++++++-- 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/core/java/com/android/internal/os/BinderCallsStats.java b/core/java/com/android/internal/os/BinderCallsStats.java index 1004e17e91e62..c297a1fc42799 100644 --- a/core/java/com/android/internal/os/BinderCallsStats.java +++ b/core/java/com/android/internal/os/BinderCallsStats.java @@ -1105,9 +1105,11 @@ public class BinderCallsStats implements BinderInternal.Observer { public static final String SETTINGS_MAX_CALL_STATS_KEY = "max_call_stats_count"; public static final String SETTINGS_IGNORE_BATTERY_STATUS_KEY = "ignore_battery_status"; // Settings for BinderLatencyObserver. - public static final String SETTINGS_COLLECT_LATENCY_DATA_KEY = "collect_Latency_data"; + public static final String SETTINGS_COLLECT_LATENCY_DATA_KEY = "collect_latency_data"; public static final String SETTINGS_LATENCY_OBSERVER_SAMPLING_INTERVAL_KEY = "latency_observer_sampling_interval"; + public static final String SETTINGS_LATENCY_OBSERVER_SHARDING_MODULO_KEY = + "latency_observer_sharding_modulo"; public static final String SETTINGS_LATENCY_OBSERVER_PUSH_INTERVAL_MINUTES_KEY = "latency_observer_push_interval_minutes"; public static final String SETTINGS_LATENCY_HISTOGRAM_BUCKET_COUNT_KEY = @@ -1187,6 +1189,9 @@ public class BinderCallsStats implements BinderInternal.Observer { binderLatencyObserver.setSamplingInterval(mParser.getInt( SETTINGS_LATENCY_OBSERVER_SAMPLING_INTERVAL_KEY, BinderLatencyObserver.PERIODIC_SAMPLING_INTERVAL_DEFAULT)); + binderLatencyObserver.setShardingModulo(mParser.getInt( + SETTINGS_LATENCY_OBSERVER_SHARDING_MODULO_KEY, + BinderLatencyObserver.SHARDING_MODULO_DEFAULT)); binderLatencyObserver.setHistogramBucketsParams( mParser.getInt( SETTINGS_LATENCY_HISTOGRAM_BUCKET_COUNT_KEY, diff --git a/core/java/com/android/internal/os/BinderLatencyObserver.java b/core/java/com/android/internal/os/BinderLatencyObserver.java index 5fa96bd9a1529..ed7e172e8071f 100644 --- a/core/java/com/android/internal/os/BinderLatencyObserver.java +++ b/core/java/com/android/internal/os/BinderLatencyObserver.java @@ -44,6 +44,7 @@ public class BinderLatencyObserver { // Latency observer parameters. public static final int PERIODIC_SAMPLING_INTERVAL_DEFAULT = 10; + public static final int SHARDING_MODULO_DEFAULT = 1; public static final int STATSD_PUSH_INTERVAL_MINUTES_DEFAULT = 360; // Histogram buckets parameters. @@ -58,6 +59,11 @@ public class BinderLatencyObserver { // Sampling period to control how often to track CPU usage. 1 means all calls, 100 means ~1 out // of 100 requests. private int mPeriodicSamplingInterval = PERIODIC_SAMPLING_INTERVAL_DEFAULT; + // Controls how many APIs will be collected per device. 1 means all APIs, 10 means every 10th + // API will be collected. + private int mShardingModulo = SHARDING_MODULO_DEFAULT; + // Controls which shards will be collected on this device. + private int mShardingOffset; private int mBucketCount = BUCKET_COUNT_DEFAULT; private int mFirstBucketSize = FIRST_BUCKET_SIZE_DEFAULT; @@ -156,7 +162,7 @@ public class BinderLatencyObserver { FrameworkStatsLog.BINDER_LATENCY_REPORTED, atom.getBytes(), mPeriodicSamplingInterval, - 1, + mShardingModulo, mBucketCount, mFirstBucketSize, mBucketScaleFactor); @@ -185,6 +191,7 @@ public class BinderLatencyObserver { mLatencyBuckets = new BinderLatencyBuckets( mBucketCount, mFirstBucketSize, mBucketScaleFactor); mProcessSource = processSource; + mShardingOffset = mRandom.nextInt(mShardingModulo); noteLatencyDelayed(); } @@ -194,7 +201,12 @@ public class BinderLatencyObserver { return; } - LatencyDims dims = new LatencyDims(s.binderClass, s.transactionCode); + LatencyDims dims = LatencyDims.create(s.binderClass, s.transactionCode); + + if (!shouldCollect(dims)) { + return; + } + long elapsedTimeMicro = getElapsedRealtimeMicro(); long callDuration = elapsedTimeMicro - s.timeStarted; @@ -220,6 +232,10 @@ public class BinderLatencyObserver { return SystemClock.elapsedRealtimeNanos() / 1000; } + protected boolean shouldCollect(LatencyDims dims) { + return (dims.hashCode() + mShardingOffset) % mShardingModulo == 0; + } + protected boolean shouldKeepSample() { return mRandom.nextInt() % mPeriodicSamplingInterval == 0; } @@ -240,6 +256,23 @@ public class BinderLatencyObserver { } } + /** Updates the sharding modulo. */ + public void setShardingModulo(int shardingModulo) { + if (shardingModulo <= 0) { + Slog.w(TAG, "Ignored invalid sharding modulo (value must be positive): " + + shardingModulo); + return; + } + + synchronized (mLock) { + if (shardingModulo != mShardingModulo) { + mShardingModulo = shardingModulo; + mShardingOffset = mRandom.nextInt(shardingModulo); + reset(); + } + } + } + /** Updates the statsd push interval. */ public void setPushInterval(int pushIntervalMinutes) { if (pushIntervalMinutes <= 0) { @@ -289,7 +322,12 @@ public class BinderLatencyObserver { // Cached hash code, 0 if not set yet. private int mHashCode = 0; - public LatencyDims(Class binderClass, int transactionCode) { + /** Creates a new instance of LatencyDims. */ + public static LatencyDims create(Class binderClass, int transactionCode) { + return new LatencyDims(binderClass, transactionCode); + } + + private LatencyDims(Class binderClass, int transactionCode) { this.mBinderClass = binderClass; this.mTransactionCode = transactionCode; } @@ -317,7 +355,7 @@ public class BinderLatencyObserver { return mHashCode; } int hash = mTransactionCode; - hash = 31 * hash + mBinderClass.hashCode(); + hash = 31 * hash + mBinderClass.getName().hashCode(); mHashCode = hash; return hash; } diff --git a/core/tests/coretests/src/com/android/internal/os/BinderLatencyObserverTest.java b/core/tests/coretests/src/com/android/internal/os/BinderLatencyObserverTest.java index 4157f5e728717..5af7376dc1327 100644 --- a/core/tests/coretests/src/com/android/internal/os/BinderLatencyObserverTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BinderLatencyObserverTest.java @@ -41,6 +41,7 @@ import org.junit.runner.RunWith; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.Random; @SmallTest @@ -71,9 +72,9 @@ public class BinderLatencyObserverTest { ArrayMap latencyHistograms = blo.getLatencyHistograms(); assertEquals(2, latencyHistograms.keySet().size()); - assertThat(latencyHistograms.get(new LatencyDims(binder.getClass(), 1))) + assertThat(latencyHistograms.get(LatencyDims.create(binder.getClass(), 1))) .asList().containsExactly(2, 0, 1, 0, 0).inOrder(); - assertThat(latencyHistograms.get(new LatencyDims(binder.getClass(), 2))) + assertThat(latencyHistograms.get(LatencyDims.create(binder.getClass(), 2))) .asList().containsExactly(0, 0, 0, 0, 2).inOrder(); } @@ -115,7 +116,7 @@ public class BinderLatencyObserverTest { // The long call should be capped to maxint (to not overflow) and placed in the last bucket. assertThat(blo.getLatencyHistograms() - .get(new LatencyDims(binder.getClass(), 1))) + .get(LatencyDims.create(binder.getClass(), 1))) .asList().containsExactly(0, 0, 0, 0, 1) .inOrder(); } @@ -132,7 +133,7 @@ public class BinderLatencyObserverTest { blo.setElapsedTime(2); blo.callEnded(callSession); - LatencyDims dims = new LatencyDims(binder.getClass(), 1); + LatencyDims dims = LatencyDims.create(binder.getClass(), 1); // Fill the buckets with maxint. Arrays.fill(blo.getLatencyHistograms().get(dims), Integer.MAX_VALUE); assertThat(blo.getLatencyHistograms().get(dims)) @@ -240,6 +241,61 @@ public class BinderLatencyObserverTest { .inOrder(); } + @Test + public void testSharding() { + TestBinderLatencyObserver blo = new TestBinderLatencyObserver(); + blo.setShardingModulo(2); + blo.setHistogramBucketsParams(5, 5, 1.125f); + + Binder binder = new Binder(); + CallSession callSession = new CallSession(); + callSession.binderClass = binder.getClass(); + callSession.transactionCode = 1; + blo.setElapsedTime(2); + blo.callEnded(callSession); + callSession.transactionCode = 2; + blo.setElapsedTime(4); + blo.callEnded(callSession); + callSession.transactionCode = 3; + blo.setElapsedTime(2); + blo.callEnded(callSession); + callSession.transactionCode = 4; + blo.setElapsedTime(4); + blo.callEnded(callSession); + + ArrayMap latencyHistograms = blo.getLatencyHistograms(); + Iterator iterator = latencyHistograms.keySet().iterator(); + LatencyDims dims; + + // Hash codes are not consistent per device and not mockable so the test needs to consider + // whether the hashCode of LatencyDims is odd or even and test accordingly. + if (LatencyDims.create(binder.getClass(), 0).hashCode() % 2 == 0) { + assertEquals(2, latencyHistograms.size()); + dims = iterator.next(); + assertEquals(binder.getClass(), dims.getBinderClass()); + assertEquals(1, dims.getTransactionCode()); + assertThat(latencyHistograms.get(dims)).asList().containsExactly(1, 0, 0, 0, 0) + .inOrder(); + dims = iterator.next(); + assertEquals(binder.getClass(), dims.getBinderClass()); + assertEquals(3, dims.getTransactionCode()); + assertThat(latencyHistograms.get(dims)).asList().containsExactly(1, 0, 0, 0, 0) + .inOrder(); + } else { + assertEquals(2, latencyHistograms.size()); + dims = iterator.next(); + assertEquals(binder.getClass(), dims.getBinderClass()); + assertEquals(2, dims.getTransactionCode()); + assertThat(latencyHistograms.get(dims)).asList().containsExactly(1, 0, 0, 0, 0) + .inOrder(); + dims = iterator.next(); + assertEquals(binder.getClass(), dims.getBinderClass()); + assertEquals(4, dims.getTransactionCode()); + assertThat(latencyHistograms.get(dims)).asList().containsExactly(1, 0, 0, 0, 0) + .inOrder(); + } + } + public static class TestBinderLatencyObserver extends BinderLatencyObserver { private long mElapsedTime = 0; private ArrayList mWrittenAtoms; @@ -259,6 +315,10 @@ public class BinderLatencyObserverTest { public int nextInt() { return mCallCount++; } + + public int nextInt(int x) { + return 1; + } }; } },