From 03e689db51284d37145b8af2e750d6bbd22312b2 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Wed, 2 Mar 2016 16:17:38 -0800 Subject: Code cleanup on NMPS and NMS. - Always check for DBG before using Slog.d. - Logs duration of NMS.systemReady() when debugging. - Logs duration of NMPS.updateRulesForGlobalChangeLocked() when debugging. - Removed redundant toString() calls. - Catch multiple exceptions. - Replaced enhanced for on nested Iterable iterations. BUG: 21725996 Change-Id: Ia775a394b59de7a0570ad4954d1fe3a2698c66d6 --- .../android/server/NetworkManagementService.java | 66 ++++++++++------------ .../server/net/NetworkPolicyManagerService.java | 27 ++++++--- 2 files changed, 51 insertions(+), 42 deletions(-) diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index 799d0bda895f..853db1722e28 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -286,8 +286,8 @@ public class NetworkManagementService extends INetworkManagementService.Stub Watchdog.getInstance().addMonitor(this); } - static NetworkManagementService create(Context context, - String socket) throws InterruptedException { + static NetworkManagementService create(Context context, String socket) + throws InterruptedException { final NetworkManagementService service = new NetworkManagementService(context, socket); final CountDownLatch connectedSignal = service.mConnectedSignal; if (DBG) Slog.d(TAG, "Creating NetworkManagementService"); @@ -303,8 +303,15 @@ public class NetworkManagementService extends INetworkManagementService.Stub } public void systemReady() { - prepareNativeDaemon(); - if (DBG) Slog.d(TAG, "Prepared"); + if (DBG) { + final long start = System.currentTimeMillis(); + prepareNativeDaemon(); + final long delta = System.currentTimeMillis() - start; + Slog.d(TAG, "Prepared in " + delta + "ms"); + return; + } else { + prepareNativeDaemon(); + } } private IBatteryStats getBatteryStats() { @@ -339,8 +346,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mObservers.getBroadcastItem(i).interfaceStatusChanged(iface, up); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -358,8 +364,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mObservers.getBroadcastItem(i).interfaceLinkStateChanged(iface, up); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -376,8 +381,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mObservers.getBroadcastItem(i).interfaceAdded(iface); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -399,8 +403,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mObservers.getBroadcastItem(i).interfaceRemoved(iface); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -417,8 +420,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mObservers.getBroadcastItem(i).limitReached(limitName, iface); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -476,8 +478,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub try { mObservers.getBroadcastItem(i).interfaceClassDataActivityChanged( Integer.toString(type), isActive, tsNanos); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -520,7 +521,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub Log.wtf(TAG, "problem enabling bandwidth controls", e); } } else { - Slog.d(TAG, "not enabling bandwidth control"); + Slog.i(TAG, "not enabling bandwidth control"); } SystemProperties.set(PROP_QTAGUID_ENABLED, mBandwidthControlEnabled ? "1" : "0"); @@ -543,7 +544,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub synchronized (mQuotaLock) { int size = mActiveQuotas.size(); if (size > 0) { - Slog.d(TAG, "Pushing " + size + " active quota rules"); + if (DBG) Slog.d(TAG, "Pushing " + size + " active quota rules"); final HashMap activeQuotas = mActiveQuotas; mActiveQuotas = Maps.newHashMap(); for (Map.Entry entry : activeQuotas.entrySet()) { @@ -553,7 +554,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub size = mActiveAlerts.size(); if (size > 0) { - Slog.d(TAG, "Pushing " + size + " active alert rules"); + if (DBG) Slog.d(TAG, "Pushing " + size + " active alert rules"); final HashMap activeAlerts = mActiveAlerts; mActiveAlerts = Maps.newHashMap(); for (Map.Entry entry : activeAlerts.entrySet()) { @@ -563,7 +564,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub size = mUidRejectOnQuota.size(); if (size > 0) { - Slog.d(TAG, "Pushing " + size + " active UID rules"); + if (DBG) Slog.d(TAG, "Pushing " + size + " active UID rules"); final SparseBooleanArray uidRejectOnQuota = mUidRejectOnQuota; mUidRejectOnQuota = new SparseBooleanArray(); for (int i = 0; i < uidRejectOnQuota.size(); i++) { @@ -573,7 +574,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub size = mUidCleartextPolicy.size(); if (size > 0) { - Slog.d(TAG, "Pushing " + size + " active UID cleartext policies"); + if (DBG) Slog.d(TAG, "Pushing " + size + " active UID cleartext policies"); final SparseIntArray local = mUidCleartextPolicy; mUidCleartextPolicy = new SparseIntArray(); for (int i = 0; i < local.size(); i++) { @@ -585,7 +586,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub size = mUidFirewallRules.size(); if (size > 0) { - Slog.d(TAG, "Pushing " + size + " active firewall UID rules"); + if (DBG) Slog.d(TAG, "Pushing " + size + " active firewall UID rules"); final SparseIntArray uidFirewallRules = mUidFirewallRules; mUidFirewallRules = new SparseIntArray(); for (int i = 0; i < uidFirewallRules.size(); i++) { @@ -596,7 +597,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub size = mUidFirewallStandbyRules.size(); if (size > 0) { - Slog.d(TAG, "Pushing " + size + " active firewall standby UID rules"); + if (DBG) Slog.d(TAG, "Pushing " + size + " active firewall standby UID rules"); final SparseIntArray uidFirewallRules = mUidFirewallStandbyRules; mUidFirewallStandbyRules = new SparseIntArray(); for (int i = 0; i < uidFirewallRules.size(); i++) { @@ -610,7 +611,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub size = mUidFirewallDozableRules.size(); if (size > 0) { - Slog.d(TAG, "Pushing " + size + " active firewall dozable UID rules"); + if (DBG) Slog.d(TAG, "Pushing " + size + " active firewall dozable UID rules"); final SparseIntArray uidFirewallRules = mUidFirewallDozableRules; mUidFirewallDozableRules = new SparseIntArray(); for (int i = 0; i < uidFirewallRules.size(); i++) { @@ -633,8 +634,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mObservers.getBroadcastItem(i).addressUpdated(iface, address); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -651,8 +651,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mObservers.getBroadcastItem(i).addressRemoved(iface, address); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -670,8 +669,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub try { mObservers.getBroadcastItem(i).interfaceDnsServerInfo(iface, lifetime, addresses); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -692,8 +690,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub } else { mObservers.getBroadcastItem(i).routeRemoved(route); } - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { @@ -1210,7 +1207,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub // TODO: remove from aidl if nobody calls externally mContext.enforceCallingOrSelfPermission(SHUTDOWN, TAG); - Slog.d(TAG, "Shutting down"); + Slog.i(TAG, "Shutting down"); } @Override @@ -2225,8 +2222,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub for (int i = 0; i < length; i++) { try { mNetworkActivityListeners.getBroadcastItem(i).onNetworkActive(); - } catch (RemoteException e) { - } catch (RuntimeException e) { + } catch (RemoteException | RuntimeException e) { } } } finally { diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java index 09b7a18af40f..9a9ca7927ce2 100644 --- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java +++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java @@ -68,6 +68,7 @@ import static android.net.wifi.WifiManager.EXTRA_NETWORK_INFO; import static android.net.wifi.WifiManager.EXTRA_WIFI_CONFIGURATION; import static android.net.wifi.WifiManager.EXTRA_WIFI_INFO; import static android.text.format.DateUtils.DAY_IN_MILLIS; + import static com.android.internal.util.ArrayUtils.appendInt; import static com.android.internal.util.Preconditions.checkNotNull; import static com.android.internal.util.XmlUtils.readBooleanAttribute; @@ -1254,8 +1255,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { } if (LOGD) { - Slog.d(TAG, "applying policy " + policy.toString() + " to ifaces " - + Arrays.toString(ifaces)); + Slog.d(TAG, "applying policy " + policy + " to ifaces " + Arrays.toString(ifaces)); } final boolean hasWarning = policy.warningBytes != LIMIT_DISABLED; @@ -2454,6 +2454,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { * {@link #mRestrictPower}, or {@link #mDeviceIdleMode} value. */ void updateRulesForGlobalChangeLocked(boolean restrictedNetworksChanged) { + long start; + if (LOGD) start = System.currentTimeMillis(); + final PackageManager pm = mContext.getPackageManager(); updateRulesForDeviceIdleLocked(); @@ -2465,8 +2468,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { PackageManager.GET_UNINSTALLED_PACKAGES | PackageManager.GET_DISABLED_COMPONENTS | PackageManager.MATCH_ENCRYPTION_AWARE_AND_UNAWARE); - for (UserInfo user : users) { - for (ApplicationInfo app : apps) { + final int usersSize = users.size(); + final int appsSize = apps.size(); + for (int i = 0; i < usersSize; i++) { + final UserInfo user = users.get(i); + for (int j = 0; j < appsSize; j++) { + final ApplicationInfo app = apps.get(j); final int uid = UserHandle.getUid(user.id, app.uid); updateRulesForUidLocked(uid); } @@ -2481,13 +2488,19 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { normalizePoliciesLocked(); updateNetworkRulesLocked(); } + if (LOGD) { + final long delta = System.currentTimeMillis() - start; + Slog.d(TAG, "updateRulesForGlobalChangeLocked(" + restrictedNetworksChanged + ") took " + + delta + "ms"); + } } void updateRulesForTempWhitelistChangeLocked() { final List users = mUserManager.getUsers(); - for (UserInfo user : users) { - for (int i = mPowerSaveTempWhitelistAppIds.size() - 1; i >= 0; i--) { - int appId = mPowerSaveTempWhitelistAppIds.keyAt(i); + for (int i = 0; i < users.size(); i++) { + final UserInfo user = users.get(i); + for (int j = mPowerSaveTempWhitelistAppIds.size() - 1; j >= 0; i--) { + int appId = mPowerSaveTempWhitelistAppIds.keyAt(j); int uid = UserHandle.getUid(user.id, appId); updateRuleForAppIdleLocked(uid); updateRuleForDeviceIdleLocked(uid); -- cgit v1.2.3-59-g8ed1b