Cache network capabilities.
Cache network capabilities to reduce the number of calls
ConnectivityController makes to ConnectivityManager.
Bug: 164460449
Bug: 166702583
Test: atest com.android.server.job.controllers.ConnectivityControllerTest
Test: atest CtsJobSchedulerTestCases
Change-Id: I501e5153d7bffd2494aec484c74b9e1b6252aeeb
(cherry picked from commit 6f9ab029005993e3674f5ccaa321f9bc4467d5b8)
diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java
index bb94275..ba83483 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java
@@ -22,6 +22,7 @@
import static com.android.server.job.JobSchedulerService.RESTRICTED_INDEX;
+import android.annotation.Nullable;
import android.app.job.JobInfo;
import android.net.ConnectivityManager;
import android.net.ConnectivityManager.NetworkCallback;
@@ -86,9 +87,12 @@
@GuardedBy("mLock")
private final SparseArray<ArraySet<JobStatus>> mRequestedWhitelistJobs = new SparseArray<>();
- /** List of currently available networks. */
+ /**
+ * Set of currently available networks mapped to their latest network capabilities. Cache the
+ * latest capabilities to avoid unnecessary calls into ConnectivityManager.
+ */
@GuardedBy("mLock")
- private final ArraySet<Network> mAvailableNetworks = new ArraySet<>();
+ private final ArrayMap<Network, NetworkCapabilities> mAvailableNetworks = new ArrayMap<>();
private static final int MSG_DATA_SAVER_TOGGLED = 0;
private static final int MSG_UID_RULES_CHANGES = 1;
@@ -165,9 +169,8 @@
public boolean isNetworkAvailable(JobStatus job) {
synchronized (mLock) {
for (int i = 0; i < mAvailableNetworks.size(); ++i) {
- final Network network = mAvailableNetworks.valueAt(i);
- final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(
- network);
+ final Network network = mAvailableNetworks.keyAt(i);
+ final NetworkCapabilities capabilities = mAvailableNetworks.valueAt(i);
final boolean satisfied = isSatisfied(job, network, capabilities, mConstants);
if (DEBUG) {
Slog.v(TAG, "isNetworkAvailable(" + job + ") with network " + network
@@ -427,9 +430,33 @@
return false;
}
+ @Nullable
+ private NetworkCapabilities getNetworkCapabilities(@Nullable Network network) {
+ if (network == null) {
+ return null;
+ }
+ synchronized (mLock) {
+ // There is technically a race here if the Network object is reused. This can happen
+ // only if that Network disconnects and the auto-incrementing network ID in
+ // ConnectivityService wraps. This should no longer be a concern if/when we only make
+ // use of asynchronous calls.
+ if (mAvailableNetworks.get(network) != null) {
+ return mAvailableNetworks.get(network);
+ }
+
+ // This should almost never happen because any time a new network connects, the
+ // NetworkCallback would populate mAvailableNetworks. However, it's currently necessary
+ // because we also call synchronous methods such as getActiveNetworkForUid.
+ // TODO(134978280): remove after switching to callback-based APIs
+ final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network);
+ mAvailableNetworks.put(network, capabilities);
+ return capabilities;
+ }
+ }
+
private boolean updateConstraintsSatisfied(JobStatus jobStatus) {
final Network network = mConnManager.getActiveNetworkForUid(jobStatus.getSourceUid());
- final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network);
+ final NetworkCapabilities capabilities = getNetworkCapabilities(network);
return updateConstraintsSatisfied(jobStatus, network, capabilities);
}
@@ -470,19 +497,13 @@
*/
private void updateTrackedJobs(int filterUid, Network filterNetwork) {
synchronized (mLock) {
- // Since this is a really hot codepath, temporarily cache any
- // answers that we get from ConnectivityManager.
- final ArrayMap<Network, NetworkCapabilities> networkToCapabilities = new ArrayMap<>();
-
boolean changed = false;
if (filterUid == -1) {
for (int i = mTrackedJobs.size() - 1; i >= 0; i--) {
- changed |= updateTrackedJobsLocked(mTrackedJobs.valueAt(i),
- filterNetwork, networkToCapabilities);
+ changed |= updateTrackedJobsLocked(mTrackedJobs.valueAt(i), filterNetwork);
}
} else {
- changed = updateTrackedJobsLocked(mTrackedJobs.get(filterUid),
- filterNetwork, networkToCapabilities);
+ changed = updateTrackedJobsLocked(mTrackedJobs.get(filterUid), filterNetwork);
}
if (changed) {
mStateChangedListener.onControllerStateChanged();
@@ -490,18 +511,13 @@
}
}
- private boolean updateTrackedJobsLocked(ArraySet<JobStatus> jobs, Network filterNetwork,
- ArrayMap<Network, NetworkCapabilities> networkToCapabilities) {
+ private boolean updateTrackedJobsLocked(ArraySet<JobStatus> jobs, Network filterNetwork) {
if (jobs == null || jobs.size() == 0) {
return false;
}
final Network network = mConnManager.getActiveNetworkForUid(jobs.valueAt(0).getSourceUid());
- NetworkCapabilities capabilities = networkToCapabilities.get(network);
- if (capabilities == null) {
- capabilities = mConnManager.getNetworkCapabilities(network);
- networkToCapabilities.put(network, capabilities);
- }
+ final NetworkCapabilities capabilities = getNetworkCapabilities(network);
final boolean networkMatch = (filterNetwork == null
|| Objects.equals(filterNetwork, network));
@@ -544,9 +560,9 @@
@Override
public void onAvailable(Network network) {
if (DEBUG) Slog.v(TAG, "onAvailable: " + network);
- synchronized (mLock) {
- mAvailableNetworks.add(network);
- }
+ // Documentation says not to call getNetworkCapabilities here but wait for
+ // onCapabilitiesChanged instead. onCapabilitiesChanged should be called immediately
+ // after this, so no need to update mAvailableNetworks here.
}
@Override
@@ -554,6 +570,9 @@
if (DEBUG) {
Slog.v(TAG, "onCapabilitiesChanged: " + network);
}
+ synchronized (mLock) {
+ mAvailableNetworks.put(network, capabilities);
+ }
updateTrackedJobs(-1, network);
}
@@ -630,6 +649,8 @@
pw.println("Available networks:");
pw.increaseIndent();
for (int i = 0; i < mAvailableNetworks.size(); i++) {
+ pw.print(mAvailableNetworks.keyAt(i));
+ pw.print(": ");
pw.println(mAvailableNetworks.valueAt(i));
}
pw.decreaseIndent();
@@ -667,7 +688,7 @@
mRequestedWhitelistJobs.keyAt(i));
}
for (int i = 0; i < mAvailableNetworks.size(); i++) {
- Network network = mAvailableNetworks.valueAt(i);
+ Network network = mAvailableNetworks.keyAt(i);
if (network != null) {
network.dumpDebug(proto,
StateControllerProto.ConnectivityController.AVAILABLE_NETWORKS);
