From da6020a329e7f51c2b0b431bd14035ea24f805b7 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Tue, 12 Dec 2023 17:21:01 +0800 Subject: [PATCH] [AF03] Adopt FastDataInput FastDataInput for reading persistent data is currently disabled by default. This functionality will be gradually enabled through A/B experiments to assess its impact. One-shot boot time increase up to 10 second is one expected outcome in worst case. Currently, the persistent data was loaded when the first caller query it, but in this change it will be done twice in the boot stage due to the needs of doing comparison. Test: adb shell dumpsys netstats Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.net.NetworkStatsServiceTest Test: manual test against data points under benchmark/assets/dataset Bug: 313841499 Change-Id: Ia927bd115de5453ca3ee22104f3efbf44a55c2b8 --- .../android/net/NetworkStatsCollection.java | 6 +- .../server/net/NetworkStatsService.java | 158 ++++++++++++++++-- .../server/net/NetworkStatsServiceTest.java | 102 ++++++++++- 3 files changed, 239 insertions(+), 27 deletions(-) diff --git a/framework-t/src/android/net/NetworkStatsCollection.java b/framework-t/src/android/net/NetworkStatsCollection.java index cb8328b5af..934b4c6a0e 100644 --- a/framework-t/src/android/net/NetworkStatsCollection.java +++ b/framework-t/src/android/net/NetworkStatsCollection.java @@ -985,8 +985,8 @@ public class NetworkStatsCollection implements FileRotator.Reader, FileRotator.W * @hide */ @Nullable - public static String compareStats( - NetworkStatsCollection migrated, NetworkStatsCollection legacy) { + public static String compareStats(NetworkStatsCollection migrated, + NetworkStatsCollection legacy, boolean allowKeyChange) { final Map migEntries = migrated.getEntries(); final Map legEntries = legacy.getEntries(); @@ -998,7 +998,7 @@ public class NetworkStatsCollection implements FileRotator.Reader, FileRotator.W final NetworkStatsHistory legHistory = legEntries.get(legKey); final NetworkStatsHistory migHistory = migEntries.get(legKey); - if (migHistory == null && couldKeyChangeOnImport(legKey)) { + if (migHistory == null && allowKeyChange && couldKeyChangeOnImport(legKey)) { unmatchedLegKeys.remove(legKey); continue; } diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index ce7fd7b4b6..3ac5e29620 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -44,7 +44,6 @@ import static android.net.NetworkStats.STATS_PER_UID; import static android.net.NetworkStats.TAG_ALL; import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; -import static android.net.NetworkStatsCollection.compareStats; import static android.net.NetworkStatsHistory.FIELD_ALL; import static android.net.NetworkTemplate.MATCH_MOBILE; import static android.net.NetworkTemplate.MATCH_TEST; @@ -295,6 +294,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { static final String CONFIG_ENABLE_NETWORK_STATS_EVENT_LOGGER = "enable_network_stats_event_logger"; + static final String NETSTATS_FASTDATAINPUT_TARGET_ATTEMPTS = + "netstats_fastdatainput_target_attempts"; + static final String NETSTATS_FASTDATAINPUT_SUCCESSES_COUNTER_NAME = "fastdatainput.successes"; + static final String NETSTATS_FASTDATAINPUT_FALLBACKS_COUNTER_NAME = "fastdatainput.fallbacks"; + private final Context mContext; private final NetworkStatsFactory mStatsFactory; private final AlarmManager mAlarmManager; @@ -318,6 +322,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private PersistentInt mImportLegacyAttemptsCounter = null; private PersistentInt mImportLegacySuccessesCounter = null; private PersistentInt mImportLegacyFallbacksCounter = null; + private PersistentInt mFastDataInputSuccessesCounter = null; + private PersistentInt mFastDataInputFallbacksCounter = null; @VisibleForTesting public static final String ACTION_NETWORK_STATS_POLL = @@ -694,6 +700,24 @@ public class NetworkStatsService extends INetworkStatsService.Stub { DEFAULT_NETSTATS_IMPORT_LEGACY_TARGET_ATTEMPTS); } + /** + * Get the count of using FastDataInput target attempts. + */ + public int getUseFastDataInputTargetAttempts() { + return DeviceConfigUtils.getDeviceConfigPropertyInt( + DeviceConfig.NAMESPACE_TETHERING, + NETSTATS_FASTDATAINPUT_TARGET_ATTEMPTS, 0); + } + + /** + * Compare two {@link NetworkStatsCollection} instances and returning a human-readable + * string description of difference for debugging purpose. + */ + public String compareStats(@NonNull NetworkStatsCollection a, + @NonNull NetworkStatsCollection b, boolean allowKeyChange) { + return NetworkStatsCollection.compareStats(a, b, allowKeyChange); + } + /** * Create a persistent counter for given directory and name. */ @@ -892,13 +916,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { synchronized (mStatsLock) { mSystemReady = true; - // create data recorders along with historical rotators - mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir, - true /* wipeOnError */); - mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir, - true /* wipeOnError */); - mUidTagRecorder = buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, - mStatsDir, true /* wipeOnError */); + makeRecordersLocked(); updatePersistThresholdsLocked(); @@ -963,13 +981,106 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private NetworkStatsRecorder buildRecorder( String prefix, NetworkStatsSettings.Config config, boolean includeTags, - File baseDir, boolean wipeOnError) { + File baseDir, boolean wipeOnError, boolean useFastDataInput) { final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService( Context.DROPBOX_SERVICE); return new NetworkStatsRecorder(new FileRotator( baseDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis), mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags, - wipeOnError, false /* useFastDataInput */, baseDir); + wipeOnError, useFastDataInput, baseDir); + } + + @GuardedBy("mStatsLock") + private void makeRecordersLocked() { + boolean useFastDataInput = true; + try { + mFastDataInputSuccessesCounter = mDeps.createPersistentCounter(mStatsDir.toPath(), + NETSTATS_FASTDATAINPUT_SUCCESSES_COUNTER_NAME); + mFastDataInputFallbacksCounter = mDeps.createPersistentCounter(mStatsDir.toPath(), + NETSTATS_FASTDATAINPUT_FALLBACKS_COUNTER_NAME); + } catch (IOException e) { + Log.wtf(TAG, "Failed to create persistent counters, skip.", e); + useFastDataInput = false; + } + + final int targetAttempts = mDeps.getUseFastDataInputTargetAttempts(); + int successes = 0; + int fallbacks = 0; + try { + successes = mFastDataInputSuccessesCounter.get(); + // Fallbacks counter would be set to non-zero value to indicate the reading was + // not successful. + fallbacks = mFastDataInputFallbacksCounter.get(); + } catch (IOException e) { + Log.wtf(TAG, "Failed to read counters, skip.", e); + useFastDataInput = false; + } + + final boolean doComparison; + if (useFastDataInput) { + // Use FastDataInput if it needs to be evaluated or at least one success. + doComparison = targetAttempts > successes + fallbacks; + // Set target attempt to -1 as the kill switch to disable the feature. + useFastDataInput = targetAttempts >= 0 && (doComparison || successes > 0); + } else { + // useFastDataInput is false due to previous failures. + doComparison = false; + } + + // create data recorders along with historical rotators. + // Don't wipe on error if comparison is needed. + mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir, + !doComparison /* wipeOnError */, useFastDataInput); + mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir, + !doComparison /* wipeOnError */, useFastDataInput); + mUidTagRecorder = buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, + mStatsDir, !doComparison /* wipeOnError */, useFastDataInput); + + if (!doComparison) return; + + final MigrationInfo[] migrations = new MigrationInfo[]{ + new MigrationInfo(mXtRecorder), + new MigrationInfo(mUidRecorder), + new MigrationInfo(mUidTagRecorder) + }; + // Set wipeOnError flag false so the recorder won't damage persistent data if reads + // failed and calling deleteAll. + final NetworkStatsRecorder[] legacyRecorders = new NetworkStatsRecorder[]{ + buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir, + false /* wipeOnError */, false /* useFastDataInput */), + buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir, + false /* wipeOnError */, false /* useFastDataInput */), + buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, mStatsDir, + false /* wipeOnError */, false /* useFastDataInput */)}; + boolean success = true; + for (int i = 0; i < migrations.length; i++) { + try { + migrations[i].collection = migrations[i].recorder.getOrLoadCompleteLocked(); + } catch (Throwable t) { + Log.wtf(TAG, "Failed to load collection, skip.", t); + success = false; + break; + } + if (!compareImportedToLegacyStats(migrations[i], legacyRecorders[i], + false /* allowKeyChange */)) { + success = false; + break; + } + } + + try { + if (success) { + mFastDataInputSuccessesCounter.set(successes + 1); + } else { + // Fallback. + mXtRecorder = legacyRecorders[0]; + mUidRecorder = legacyRecorders[1]; + mUidTagRecorder = legacyRecorders[2]; + mFastDataInputFallbacksCounter.set(fallbacks + 1); + } + } catch (IOException e) { + Log.wtf(TAG, "Failed to update counters. success = " + success, e); + } } @GuardedBy("mStatsLock") @@ -1068,7 +1179,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { new NetworkStatsSettings.Config(HOUR_IN_MILLIS, 15 * DAY_IN_MILLIS, 90 * DAY_IN_MILLIS); final NetworkStatsRecorder devRecorder = buildRecorder(PREFIX_DEV, devConfig, - false, mStatsDir, true /* wipeOnError */); + false, mStatsDir, true /* wipeOnError */, false /* useFastDataInput */); final MigrationInfo[] migrations = new MigrationInfo[]{ new MigrationInfo(devRecorder), new MigrationInfo(mXtRecorder), new MigrationInfo(mUidRecorder), new MigrationInfo(mUidTagRecorder) @@ -1085,11 +1196,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { legacyRecorders = new NetworkStatsRecorder[]{ null /* dev Recorder */, buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, legacyBaseDir, - false /* wipeOnError */), + false /* wipeOnError */, false /* useFastDataInput */), buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, legacyBaseDir, - false /* wipeOnError */), + false /* wipeOnError */, false /* useFastDataInput */), buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, legacyBaseDir, - false /* wipeOnError */)}; + false /* wipeOnError */, false /* useFastDataInput */)}; } else { legacyRecorders = null; } @@ -1120,7 +1231,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { if (runComparison) { final boolean success = - compareImportedToLegacyStats(migration, legacyRecorders[i]); + compareImportedToLegacyStats(migration, legacyRecorders[i], + true /* allowKeyChange */); if (!success && !dryRunImportOnly) { tryIncrementLegacyFallbacksCounter(); } @@ -1243,7 +1355,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { * does not match or throw with exceptions. */ private boolean compareImportedToLegacyStats(@NonNull MigrationInfo migration, - @Nullable NetworkStatsRecorder legacyRecorder) { + @Nullable NetworkStatsRecorder legacyRecorder, boolean allowKeyChange) { final NetworkStatsCollection legacyStats; // Skip the recorder that doesn't need to be compared. if (legacyRecorder == null) return true; @@ -1258,7 +1370,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // The result of comparison is only for logging. try { - final String error = compareStats(migration.collection, legacyStats); + final String error = mDeps.compareStats(migration.collection, legacyStats, + allowKeyChange); if (error != null) { Log.wtf(TAG, "Unexpected comparison result for recorder " + legacyRecorder.getCookie() + ": " + error); @@ -2639,6 +2752,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } pw.println(CONFIG_ENABLE_NETWORK_STATS_EVENT_LOGGER + ": " + mSupportEventLogger); + pw.print(NETSTATS_FASTDATAINPUT_TARGET_ATTEMPTS, + mDeps.getUseFastDataInputTargetAttempts()); + pw.println(); + try { + pw.print("FastDataInput successes", mFastDataInputSuccessesCounter.get()); + pw.println(); + pw.print("FastDataInput fallbacks", mFastDataInputFallbacksCounter.get()); + pw.println(); + } catch (IOException e) { + pw.println("(failed to dump FastDataInput counters)"); + } pw.decreaseIndent(); diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index 9e6beb87a8..1ee3f9dede 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -67,6 +67,8 @@ import static android.text.format.DateUtils.WEEK_IN_MILLIS; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_RAT_CHANGED; import static com.android.server.net.NetworkStatsEventLogger.PollEvent.pollReasonNameOf; import static com.android.server.net.NetworkStatsService.ACTION_NETWORK_STATS_POLL; +import static com.android.server.net.NetworkStatsService.NETSTATS_FASTDATAINPUT_FALLBACKS_COUNTER_NAME; +import static com.android.server.net.NetworkStatsService.NETSTATS_FASTDATAINPUT_SUCCESSES_COUNTER_NAME; import static com.android.server.net.NetworkStatsService.NETSTATS_IMPORT_ATTEMPTS_COUNTER_NAME; import static com.android.server.net.NetworkStatsService.NETSTATS_IMPORT_FALLBACKS_COUNTER_NAME; import static com.android.server.net.NetworkStatsService.NETSTATS_IMPORT_SUCCESSES_COUNTER_NAME; @@ -283,9 +285,14 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private @Mock PersistentInt mImportLegacyAttemptsCounter; private @Mock PersistentInt mImportLegacySuccessesCounter; private @Mock PersistentInt mImportLegacyFallbacksCounter; + private int mFastDataInputTargetAttempts = 0; + private @Mock PersistentInt mFastDataInputSuccessesCounter; + private @Mock PersistentInt mFastDataInputFallbacksCounter; + private String mCompareStatsResult = null; private @Mock Resources mResources; private Boolean mIsDebuggable; private HandlerThread mObserverHandlerThread; + final TestDependencies mDeps = new TestDependencies(); private class MockContext extends BroadcastInterceptingContext { private final Context mBaseContext; @@ -368,7 +375,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG); mHandlerThread = new HandlerThread("NetworkStatsServiceTest-HandlerThread"); - final NetworkStatsService.Dependencies deps = makeDependencies(); // Create a separate thread for observers to run on. This thread cannot be the same // as the handler thread, because the observer callback is fired on this thread, and // it should not be blocked by client code. Additionally, creating the observers @@ -383,7 +389,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } }; mService = new NetworkStatsService(mServiceContext, mNetd, mAlarmManager, wakeLock, - mClock, mSettings, mStatsFactory, statsObservers, deps); + mClock, mSettings, mStatsFactory, statsObservers, mDeps); mElapsedRealtime = 0L; @@ -422,12 +428,9 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { mUsageCallback = new TestableUsageCallback(mUsageCallbackBinder); } - @NonNull - private TestDependencies makeDependencies() { - return new TestDependencies(); - } - class TestDependencies extends NetworkStatsService.Dependencies { + private int mCompareStatsInvocation = 0; + @Override public File getLegacyStatsDir() { return mLegacyStatsDir; @@ -448,6 +451,22 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { return mImportLegacyTargetAttempts; } + @Override + public int getUseFastDataInputTargetAttempts() { + return mFastDataInputTargetAttempts; + } + + @Override + public String compareStats(NetworkStatsCollection a, NetworkStatsCollection b, + boolean allowKeyChange) { + mCompareStatsInvocation++; + return mCompareStatsResult; + } + + int getCompareStatsInvocation() { + return mCompareStatsInvocation; + } + @Override public PersistentInt createPersistentCounter(@NonNull Path dir, @NonNull String name) { switch (name) { @@ -457,6 +476,10 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { return mImportLegacySuccessesCounter; case NETSTATS_IMPORT_FALLBACKS_COUNTER_NAME: return mImportLegacyFallbacksCounter; + case NETSTATS_FASTDATAINPUT_SUCCESSES_COUNTER_NAME: + return mFastDataInputSuccessesCounter; + case NETSTATS_FASTDATAINPUT_FALLBACKS_COUNTER_NAME: + return mFastDataInputFallbacksCounter; default: throw new IllegalArgumentException("Unknown counter name: " + name); } @@ -2165,6 +2188,71 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } } + @Test + public void testAdoptFastDataInput_featureDisabled() throws Exception { + // Boot through serviceReady() with flag disabled, verify the persistent + // counters are not increased. + mFastDataInputTargetAttempts = 0; + doReturn(0).when(mFastDataInputSuccessesCounter).get(); + doReturn(0).when(mFastDataInputFallbacksCounter).get(); + mService.systemReady(); + verify(mFastDataInputSuccessesCounter, never()).set(anyInt()); + verify(mFastDataInputFallbacksCounter, never()).set(anyInt()); + assertEquals(0, mDeps.getCompareStatsInvocation()); + } + + @Test + public void testAdoptFastDataInput_noRetryAfterFail() throws Exception { + // Boot through serviceReady(), verify the service won't retry unexpectedly + // since the target attempt remains the same. + mFastDataInputTargetAttempts = 1; + doReturn(0).when(mFastDataInputSuccessesCounter).get(); + doReturn(1).when(mFastDataInputFallbacksCounter).get(); + mService.systemReady(); + verify(mFastDataInputSuccessesCounter, never()).set(anyInt()); + verify(mFastDataInputFallbacksCounter, never()).set(anyInt()); + } + + @Test + public void testAdoptFastDataInput_noRetryAfterSuccess() throws Exception { + // Boot through serviceReady(), verify the service won't retry unexpectedly + // since the target attempt remains the same. + mFastDataInputTargetAttempts = 1; + doReturn(1).when(mFastDataInputSuccessesCounter).get(); + doReturn(0).when(mFastDataInputFallbacksCounter).get(); + mService.systemReady(); + verify(mFastDataInputSuccessesCounter, never()).set(anyInt()); + verify(mFastDataInputFallbacksCounter, never()).set(anyInt()); + } + + @Test + public void testAdoptFastDataInput_hasDiff() throws Exception { + // Boot through serviceReady() with flag enabled and assumes the stats are + // failed to compare, verify the fallbacks counter is increased. + mockDefaultSettings(); + doReturn(0).when(mFastDataInputSuccessesCounter).get(); + doReturn(0).when(mFastDataInputFallbacksCounter).get(); + mFastDataInputTargetAttempts = 1; + mCompareStatsResult = "Has differences"; + mService.systemReady(); + verify(mFastDataInputSuccessesCounter, never()).set(anyInt()); + verify(mFastDataInputFallbacksCounter).set(1); + } + + @Test + public void testAdoptFastDataInput_noDiff() throws Exception { + // Boot through serviceReady() with target attempts increased, + // assumes there was a previous failure, + // and assumes the stats are successfully compared, + // verify the successes counter is increased. + mFastDataInputTargetAttempts = 2; + doReturn(1).when(mFastDataInputFallbacksCounter).get(); + mCompareStatsResult = null; + mService.systemReady(); + verify(mFastDataInputSuccessesCounter).set(1); + verify(mFastDataInputFallbacksCounter, never()).set(anyInt()); + } + @Test public void testStatsFactoryRemoveUids() throws Exception { // pretend that network comes online