From e574a37db187958d57ca2733f40da840ea472a45 Mon Sep 17 00:00:00 2001
From: Winson Chiu <chiuwinson@google.com>
Date: Wed, 15 Jun 2022 15:40:08 +0000
Subject: [PATCH] Fix high traffic PMS lockless methods

The notifyPackageUse method is called so many times that it is
causing a spike in snapshot invalidations. This reverts to a
(broken) S state where the code takes the lock and updates
the time value, but doesn't actually invalidate the snapshot,
preventing the re-generation.

Also fixes a check in grantImplicitAccess that was calling
onChanged() even though nothing changed, which was the other
significant method.

Test: manual, run profiling steps in bug on pre-change and post-change
Test: atest AppsFilterImplTest

Bug: 236021119

Change-Id: I20b1bb1d8b4f3df09c901f39db8462ab1a67607d
---
 .../java/com/android/server/pm/AppsFilterImpl.java    |  4 +++-
 .../com/android/server/pm/PackageManagerService.java  | 11 +++++++----
 .../server/pm/pkg/PackageStateUnserialized.java       |  5 ++++-
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/services/core/java/com/android/server/pm/AppsFilterImpl.java b/services/core/java/com/android/server/pm/AppsFilterImpl.java
index 9fddc76b78c3e..181c39ee50b50 100644
--- a/services/core/java/com/android/server/pm/AppsFilterImpl.java
+++ b/services/core/java/com/android/server/pm/AppsFilterImpl.java
@@ -418,7 +418,9 @@ public final class AppsFilterImpl extends AppsFilterLocked implements Watchable,
         } else if (changed) {
             invalidateCache("grantImplicitAccess: " + recipientUid + " -> " + visibleUid);
         }
-        onChanged();
+        if (changed) {
+            onChanged();
+        }
         return changed;
     }
 
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 4f8c792d3b495..94e8ec5c434d4 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -2859,12 +2859,15 @@ public class PackageManagerService implements PackageSender, TestUtilityService
         mDexOptHelper.performPackageDexOptUpgradeIfNeeded();
     }
 
-
     private void notifyPackageUseInternal(String packageName, int reason) {
         long time = System.currentTimeMillis();
-        commitPackageStateMutation(null, packageName, packageState -> {
-            packageState.setLastPackageUsageTime(reason, time);
-        });
+        synchronized (mLock) {
+            final PackageSetting pkgSetting = mSettings.getPackageLPr(packageName);
+            if (pkgSetting == null) {
+                return;
+            }
+            pkgSetting.getPkgState().setLastPackageUsageTimeInMills(reason, time);
+        }
     }
 
     /*package*/ DexManager getDexManager() {
diff --git a/services/core/java/com/android/server/pm/pkg/PackageStateUnserialized.java b/services/core/java/com/android/server/pm/pkg/PackageStateUnserialized.java
index 7bd720acc7997..5251fe026a9b0 100644
--- a/services/core/java/com/android/server/pm/pkg/PackageStateUnserialized.java
+++ b/services/core/java/com/android/server/pm/pkg/PackageStateUnserialized.java
@@ -79,7 +79,10 @@ public class PackageStateUnserialized {
             return this;
         }
         getLastPackageUsageTimeInMills()[reason] = time;
-        mPackageSetting.onChanged();
+        // TODO(b/236180425): This method does not notify snapshot changes because it's called too
+        //  frequently, causing too many re-takes. This should be moved to a separate data structure
+        //  or merged with the general UsageStats to avoid tracking heavily mutated data in the
+        //  package data snapshot.
         return this;
     }