From bbecd0e7e6c17f56af80318877243ac80198aa4a Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Sat, 7 Nov 2020 01:07:46 +0000 Subject: [PATCH 1/2] Revert "Refresh legacy state before writing to runtime permission persistence." This reverts commit fea3a1709f313134da9ca222c6e39a40ad409641. Reason for revert: Performance regression Fixes: 172297495 Test: presubmit Change-Id: I286015aafedfb0807392f8b31476b083c82bc086 --- .../android/server/pm/PackageManagerService.java | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index f68469fee4da5..7afd3dbb2c1ef 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -19606,7 +19606,7 @@ public class PackageManagerService extends IPackageManager.Stub ps.setUninstallReason(UNINSTALL_REASON_UNKNOWN, userId); } - writeRuntimePermissionsForUserLPrTEMP(userId, false); + mSettings.writeRuntimePermissionsForUserLPr(userId, false); } // Regardless of writeSettings we need to ensure that this restriction // state propagation is persisted @@ -25752,7 +25752,7 @@ public class PackageManagerService extends IPackageManager.Stub public void writePermissionSettings(int[] userIds, boolean async) { synchronized (mLock) { for (int userId : userIds) { - writeRuntimePermissionsForUserLPrTEMP(userId, !async); + mSettings.writeRuntimePermissionsForUserLPr(userId, !async); } } } @@ -26401,17 +26401,6 @@ public class PackageManagerService extends IPackageManager.Stub mSettings.writeLPr(); } - /** - * Temporary method that wraps mSettings.writeRuntimePermissionsForUserLPr() and calls - * mPermissionManager.writeLegacyPermissionStateTEMP() beforehand. - * - * TODO(zhanghai): This should be removed once we finish migration of permission storage. - */ - private void writeRuntimePermissionsForUserLPrTEMP(@UserIdInt int userId, boolean async) { - mPermissionManager.writeLegacyPermissionStateTEMP(); - mSettings.writeRuntimePermissionsForUserLPr(userId, async); - } - @Override public IBinder getHoldLockToken() { if (!Build.IS_DEBUGGABLE) { From f9c011501fab1f9ae63c2535e91d1e7d823f5ff0 Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Fri, 6 Nov 2020 16:42:33 -0800 Subject: [PATCH 2/2] Refresh legacy permission state right before writing. This was done for writing all the package settings, but not when writing runtime permissions only, so we need to do the same thing before we eventually migrate to new persistence. This is a resubmission of ag/12960409, and this time we are only refreshing the state before the actual disk write, instead of whenever a write is (asynchronously) scheduled. Bug: 171755668 Bug: 172297495 Test: manual Test: atest MultiUserPerfTests Change-Id: I0c69b6a28ea795c6682e28a26be4cac5b644c70b --- services/core/java/com/android/server/pm/Settings.java | 2 ++ .../pm/permission/LegacyPermissionDataProvider.java | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index cd9a4e72672fd..71e7358701fd0 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -5390,6 +5390,8 @@ public final class Settings { mHandler.removeMessages(userId); mWriteScheduled.delete(userId); + mPermissionDataProvider.writeLegacyPermissionStateTEMP(); + int version = mVersions.get(userId, INITIAL_VERSION); String fingerprint = mFingerprints.get(userId); diff --git a/services/core/java/com/android/server/pm/permission/LegacyPermissionDataProvider.java b/services/core/java/com/android/server/pm/permission/LegacyPermissionDataProvider.java index 0e790b1899edf..46e4e59bb15bd 100644 --- a/services/core/java/com/android/server/pm/permission/LegacyPermissionDataProvider.java +++ b/services/core/java/com/android/server/pm/permission/LegacyPermissionDataProvider.java @@ -61,4 +61,14 @@ public interface LegacyPermissionDataProvider { */ @NonNull int[] getGidsForUid(int uid); + + /** + * This method should be in PermissionManagerServiceInternal, however it is made available here + * as well to avoid serious performance regression in writePermissionSettings(), which seems to + * be a hot spot and we should delay calling this method until wre are actually writing the + * file, instead of every time an async write is requested. + * + * @see PermissionManagerServiceInternal#writeLegacyPermissionStateTEMP() + */ + void writeLegacyPermissionStateTEMP(); }