From d0e049455cdfcbf30455d21dd337e0cd306deb80 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 10 Jul 2024 18:10:05 +0800 Subject: [PATCH] Support NetworkStats#addEntries API Currently, NetworkStats#addEntry creates a new instance containing the requested result instead of mutating the original object. This creates a heavy memory footprint when the caller invokes it in a loop because a new NetworkStats instance is created whenever an addEntry is called. This change introduces a new API that allows callers to add a set of entries to a NetworkStats instance. This new API will clone the instance only once, return the newly created (and modified) instance, and ensure that the original instance remains immutable. Additionally, the change adopts config libraries for flagging the API and corresponding CTS tests, since Android U-based branches are no longer downstream of current development branches. Test: atest FrameworksNetTests:android.net.connectivity.android.net.netstats.NetworkStatsApiTest Test: atest CtsNetTestCases:NetworkStatsApiTest Bug: 335680025 Fix: 357772271 API-Coverage-Bug: 357772271 Change-Id: I58513844e681c0621a6107ec4b198cd3957f289d --- common/FlaggedApi.bp | 10 +++ common/flags.aconfig | 8 ++ framework-t/Android.bp | 4 + framework-t/api/system-current.txt | 1 + framework-t/src/android/net/NetworkStats.java | 17 +++++ framework/Android.bp | 1 + .../net/netstats/NetworkStatsApiTest.kt | 76 ++++++++++++------- 7 files changed, 90 insertions(+), 27 deletions(-) diff --git a/common/FlaggedApi.bp b/common/FlaggedApi.bp index 21be1d3d5e..d5cfde3717 100644 --- a/common/FlaggedApi.bp +++ b/common/FlaggedApi.bp @@ -22,6 +22,16 @@ aconfig_declarations { visibility: ["//packages/modules/Connectivity:__subpackages__"], } +java_aconfig_library { + name: "com.android.net.flags-aconfig-java", + aconfig_declarations: "com.android.net.flags-aconfig", + defaults: ["framework-minus-apex-aconfig-java-defaults"], + min_sdk_version: "30", + apex_available: [ + "com.android.tethering", + ], +} + aconfig_declarations { name: "com.android.net.thread.flags-aconfig", package: "com.android.net.thread.flags", diff --git a/common/flags.aconfig b/common/flags.aconfig index b320b61ee7..1b0da4ea94 100644 --- a/common/flags.aconfig +++ b/common/flags.aconfig @@ -123,3 +123,11 @@ flag { description: "Flag for introducing TETHERING_VIRTUAL type" bug: "340376953" } + +flag { + name: "netstats_add_entries" + is_exported: true + namespace: "android_core_networking" + description: "Flag for NetworkStats#addEntries API" + bug: "335680025" +} diff --git a/framework-t/Android.bp b/framework-t/Android.bp index f076f5bcf2..ac78d096e5 100644 --- a/framework-t/Android.bp +++ b/framework-t/Android.bp @@ -137,6 +137,10 @@ java_sdk_library { // framework-connectivity-pre-jarjar match at runtime. jarjar_rules: ":framework-connectivity-jarjar-rules", stub_only_libs: [ + // static_libs is not used to compile stubs. So libs which have + // been included in static_libs might still need to + // be in stub_only_libs to be usable when generating the API stubs. + "com.android.net.flags-aconfig-java", // Use prebuilt framework-connectivity stubs to avoid circular dependencies "sdk_module-lib_current_framework-connectivity", ], diff --git a/framework-t/api/system-current.txt b/framework-t/api/system-current.txt index 52de1a39e2..2354882b62 100644 --- a/framework-t/api/system-current.txt +++ b/framework-t/api/system-current.txt @@ -310,6 +310,7 @@ package android.net { public final class NetworkStats implements java.lang.Iterable android.os.Parcelable { ctor public NetworkStats(long, int); method @NonNull public android.net.NetworkStats add(@NonNull android.net.NetworkStats); + method @FlaggedApi("com.android.net.flags.netstats_add_entries") @NonNull public android.net.NetworkStats addEntries(@NonNull java.util.List); method @NonNull public android.net.NetworkStats addEntry(@NonNull android.net.NetworkStats.Entry); method public android.net.NetworkStats clone(); method public int describeContents(); diff --git a/framework-t/src/android/net/NetworkStats.java b/framework-t/src/android/net/NetworkStats.java index e9a3f5839c..a2c4fc374e 100644 --- a/framework-t/src/android/net/NetworkStats.java +++ b/framework-t/src/android/net/NetworkStats.java @@ -18,6 +18,7 @@ package android.net; import static com.android.net.module.util.NetworkStatsUtils.multiplySafeByRational; +import android.annotation.FlaggedApi; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -33,6 +34,7 @@ import android.util.SparseBooleanArray; import com.android.internal.annotations.VisibleForTesting; import com.android.modules.utils.build.SdkLevel; +import com.android.net.flags.Flags; import com.android.net.module.util.CollectionUtils; import libcore.util.EmptyArray; @@ -844,6 +846,21 @@ public final class NetworkStats implements Parcelable, Iterable entries) { + final NetworkStats newStats = this.clone(); + for (final Entry entry : Objects.requireNonNull(entries)) { + newStats.combineValues(entry); + } + return newStats; + } + /** * Add given values with an existing row, or create a new row if * {@link #findIndex(String, int, int, int, int, int, int)} is unable to find match. Can diff --git a/framework/Android.bp b/framework/Android.bp index 282ba4baef..deea6b6ac9 100644 --- a/framework/Android.bp +++ b/framework/Android.bp @@ -86,6 +86,7 @@ java_defaults { "framework-wifi.stubs.module_lib", ], static_libs: [ + "com.android.net.flags-aconfig-java", // Not using the latest stable version because all functions in the latest version of // mdns_aidl_interface are deprecated. "mdns_aidl_interface-V1-java", diff --git a/tests/common/java/android/net/netstats/NetworkStatsApiTest.kt b/tests/common/java/android/net/netstats/NetworkStatsApiTest.kt index 8cef6aa774..17f5e96a3e 100644 --- a/tests/common/java/android/net/netstats/NetworkStatsApiTest.kt +++ b/tests/common/java/android/net/netstats/NetworkStatsApiTest.kt @@ -28,18 +28,26 @@ import android.net.NetworkStats.ROAMING_YES import android.net.NetworkStats.SET_DEFAULT import android.net.NetworkStats.SET_FOREGROUND import android.net.NetworkStats.TAG_NONE +import android.os.Build import androidx.test.filters.SmallTest +import com.android.testutils.ConnectivityModuleTest +import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo +import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.assertNetworkStatsEquals import com.android.testutils.assertParcelingIsLossless import kotlin.test.assertEquals import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.junit.runners.JUnit4 -@RunWith(JUnit4::class) +@DevSdkIgnoreRunner.MonitorThreadLeak +@RunWith(DevSdkIgnoreRunner::class) @SmallTest class NetworkStatsApiTest { + @get:Rule + val ignoreRule = DevSdkIgnoreRule() private val testStatsEmpty = NetworkStats(0L, 0) // Note that these variables need to be initialized outside of constructor, initialize @@ -49,6 +57,7 @@ class NetworkStatsApiTest { // be merged if performing add on these 2 stats. private lateinit var testStats1: NetworkStats private lateinit var testStats2: NetworkStats + private lateinit var expectedEntriesInStats2: List // This is a result of adding stats1 and stats2, while the merging of common key items is // subject to test later, this should not be initialized with for a loop to add stats1 @@ -84,19 +93,23 @@ class NetworkStatsApiTest { METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 3, 1, 6, 2, 0)) assertEquals(8, testStats1.size()) - testStats2 = NetworkStats(0L, 0) - // Entries which are common for set1 and set2. - .addEntry(Entry(TEST_IFACE, TEST_UID1, SET_DEFAULT, 0x80, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 3, 15, 2, 31, 1)) - .addEntry(Entry(TEST_IFACE, TEST_UID1, SET_FOREGROUND, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 13, 61, 10, 1, 45)) - .addEntry(Entry(TEST_IFACE, TEST_UID2, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 11, 2, 3, 4, 7)) - .addEntry(Entry(IFACE_VT, TEST_UID1, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 4, 3, 2, 1, 0)) - // Entry which only appears in set2. - .addEntry(Entry(IFACE_VT, TEST_UID2, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 2, 3, 7, 8, 0)) + expectedEntriesInStats2 = listOf( + // Entries which are common for set1 and set2. + Entry(TEST_IFACE, TEST_UID1, SET_DEFAULT, 0x80, + METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 3, 15, 2, 31, 1), + Entry(TEST_IFACE, TEST_UID1, SET_FOREGROUND, TAG_NONE, + METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 13, 61, 10, 1, 45), + Entry(TEST_IFACE, TEST_UID2, SET_DEFAULT, TAG_NONE, + METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 11, 2, 3, 4, 7), + Entry(IFACE_VT, TEST_UID1, SET_DEFAULT, TAG_NONE, + METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 4, 3, 2, 1, 0), + // Entry which only appears in set2. + Entry(IFACE_VT, TEST_UID2, SET_DEFAULT, TAG_NONE, + METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 2, 3, 7, 8, 0)) + testStats2 = NetworkStats(0L, 5) + for (entry in expectedEntriesInStats2) { + testStats2 = testStats2.addEntry(entry) + } assertEquals(5, testStats2.size()) testStats3 = NetworkStats(0L, 9) @@ -125,18 +138,6 @@ class NetworkStatsApiTest { @Test fun testAddEntry() { - val expectedEntriesInStats2 = arrayOf( - Entry(TEST_IFACE, TEST_UID1, SET_DEFAULT, 0x80, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 3, 15, 2, 31, 1), - Entry(TEST_IFACE, TEST_UID1, SET_FOREGROUND, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 13, 61, 10, 1, 45), - Entry(TEST_IFACE, TEST_UID2, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 11, 2, 3, 4, 7), - Entry(IFACE_VT, TEST_UID1, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 4, 3, 2, 1, 0), - Entry(IFACE_VT, TEST_UID2, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 2, 3, 7, 8, 0)) - // While testStats* are already initialized with addEntry, verify content added // matches expectation. for (i in expectedEntriesInStats2.indices) { @@ -150,6 +151,27 @@ class NetworkStatsApiTest { assertEquals(Entry(IFACE_VT, TEST_UID1, SET_DEFAULT, TAG_NONE, METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 16, -2, 9, 1, 9), stats.getValues(3, null)) + + // Verify the original ststs object is not altered. + for (i in expectedEntriesInStats2.indices) { + val entry = testStats2.getValues(i, null) + assertEquals(expectedEntriesInStats2[i], entry) + } + } + + @ConnectivityModuleTest + @IgnoreUpTo(Build.VERSION_CODES.S_V2) // Mainlined NetworkStats only runs on T+ + @Test + fun testAddEntries() { + val baseStats = NetworkStats(0L, 1) + .addEntry(Entry(IFACE_VT, TEST_UID1, SET_DEFAULT, TAG_NONE, + METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 12, -5, 7, 0, 9)) + val statsUnderTest = baseStats.addEntries(expectedEntriesInStats2) + // Assume the correctness of addEntry is verified in other tests. + val expectedStats = testStats2 + .addEntry(Entry(IFACE_VT, TEST_UID1, SET_DEFAULT, TAG_NONE, + METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 12, -5, 7, 0, 9)) + assertNetworkStatsEquals(expectedStats, statsUnderTest) } @Test