From 0625939a0232905b8fa6cf67ad779ee8491d9f30 Mon Sep 17 00:00:00 2001
From: Tetiana Meronyk <tetianameronyk@google.com>
Date: Fri, 19 Apr 2024 20:25:15 +0000
Subject: [PATCH] Only rewrite file when there has been a change to a list of
users
Due to I/O operations being performed on BG thread, every call, especially during user boot, adds to the overall performance impact. This change adds extra checks to ensure writing only happens when the change occured.
Bug: 331980379
Bug: 335667295
Test: perfetto trace && manual
Change-Id: I86c04ef7307b10e6336b80699c1091f107d8d1bc
---
.../android/server/alarm/UserWakeupStore.java | 53 ++++++++++++++-----
.../server/alarm/UserWakeupStoreTest.java | 26 ++++-----
2 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java
index a0d9133b93da..7fc630c964e5 100644
--- a/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java
+++ b/apex/jobscheduler/service/java/com/android/server/alarm/UserWakeupStore.java
@@ -136,10 +136,9 @@ public class UserWakeupStore {
* Remove wakeup scheduled for the user with given userId if present.
*/
public void removeUserWakeup(int userId) {
- synchronized (mUserWakeupLock) {
- mUserStarts.delete(userId);
+ if (deleteWakeupFromUserStarts(userId)) {
+ updateUserListFile();
}
- updateUserListFile();
}
/**
@@ -186,7 +185,7 @@ public class UserWakeupStore {
* Return scheduled start time for user or -1 if user does not have alarm set.
*/
@VisibleForTesting
- long getWakeupTimeForUserForTest(int userId) {
+ long getWakeupTimeForUser(int userId) {
synchronized (mUserWakeupLock) {
return mUserStarts.get(userId, -1);
}
@@ -197,8 +196,11 @@ public class UserWakeupStore {
*/
public void onUserStarting(int userId) {
synchronized (mUserWakeupLock) {
- mStartingUsers.put(userId, getWakeupTimeForUserForTest(userId));
- mUserStarts.delete(userId);
+ final long wakeup = getWakeupTimeForUser(userId);
+ if (wakeup >= 0) {
+ mStartingUsers.put(userId, wakeup);
+ mUserStarts.delete(userId);
+ }
}
}
@@ -206,21 +208,48 @@ public class UserWakeupStore {
* Remove userId from starting user list once start is complete.
*/
public void onUserStarted(int userId) {
- synchronized (mUserWakeupLock) {
- mStartingUsers.delete(userId);
+ if (deleteWakeupFromStartingUsers(userId)) {
+ updateUserListFile();
}
- updateUserListFile();
}
/**
* Remove userId from the store when the user is removed.
*/
public void onUserRemoved(int userId) {
+ if (deleteWakeupFromUserStarts(userId) || deleteWakeupFromStartingUsers(userId)) {
+ updateUserListFile();
+ }
+ }
+
+ /**
+ * Remove wakeup for a given userId from mUserStarts.
+ * @return true if an entry is found and the list of wakeups changes.
+ */
+ private boolean deleteWakeupFromUserStarts(int userId) {
+ int index;
synchronized (mUserWakeupLock) {
- mUserStarts.delete(userId);
- mStartingUsers.delete(userId);
+ index = mUserStarts.indexOfKey(userId);
+ if (index >= 0) {
+ mUserStarts.removeAt(index);
+ }
}
- updateUserListFile();
+ return index >= 0;
+ }
+
+ /**
+ * Remove wakeup for a given userId from mStartingUsers.
+ * @return true if an entry is found and the list of wakeups changes.
+ */
+ private boolean deleteWakeupFromStartingUsers(int userId) {
+ int index;
+ synchronized (mUserWakeupLock) {
+ index = mStartingUsers.indexOfKey(userId);
+ if (index >= 0) {
+ mStartingUsers.removeAt(index);
+ }
+ }
+ return index >= 0;
}
/**
diff --git a/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java b/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java
index 5d3e4994d718..75e8e68dd98a 100644
--- a/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/alarm/UserWakeupStoreTest.java
@@ -116,7 +116,7 @@ public class UserWakeupStoreTest {
mUserWakeupStore.addUserWakeup(USER_ID_1, TEST_TIMESTAMP - 7_000);
mUserWakeupStore.addUserWakeup(USER_ID_1, finalAlarmTime);
assertEquals(1, mUserWakeupStore.getUserIdsToWakeup(TEST_TIMESTAMP).length);
- final long alarmTime = mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_1)
+ final long alarmTime = mUserWakeupStore.getWakeupTimeForUser(USER_ID_1)
+ BUFFER_TIME_MS;
assertTrue(finalAlarmTime + USER_START_TIME_DEVIATION_LIMIT_MS >= alarmTime);
assertTrue(finalAlarmTime - USER_START_TIME_DEVIATION_LIMIT_MS <= alarmTime);
@@ -129,8 +129,8 @@ public class UserWakeupStoreTest {
mUserWakeupStore.addUserWakeup(USER_ID_3, TEST_TIMESTAMP - 13_000);
assertEquals(3, mUserWakeupStore.getUserIdsToWakeup(TEST_TIMESTAMP).length);
mUserWakeupStore.removeUserWakeup(USER_ID_3);
- assertEquals(-1, mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_3));
- assertTrue(mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_2) > 0);
+ assertEquals(-1, mUserWakeupStore.getWakeupTimeForUser(USER_ID_3));
+ assertTrue(mUserWakeupStore.getWakeupTimeForUser(USER_ID_2) > 0);
}
@Test
@@ -139,10 +139,10 @@ public class UserWakeupStoreTest {
mUserWakeupStore.addUserWakeup(USER_ID_2, TEST_TIMESTAMP - 3_000);
mUserWakeupStore.addUserWakeup(USER_ID_3, TEST_TIMESTAMP - 13_000);
assertEquals(mUserWakeupStore.getNextWakeupTime(),
- mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_1));
+ mUserWakeupStore.getWakeupTimeForUser(USER_ID_1));
mUserWakeupStore.removeUserWakeup(USER_ID_1);
assertEquals(mUserWakeupStore.getNextWakeupTime(),
- mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_3));
+ mUserWakeupStore.getWakeupTimeForUser(USER_ID_3));
}
@Test
@@ -154,16 +154,16 @@ public class UserWakeupStoreTest {
mUserWakeupStore.init();
final long realtime = SystemClock.elapsedRealtime();
assertEquals(0, mUserWakeupStore.getUserIdsToWakeup(TEST_TIMESTAMP).length);
- assertTrue(mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_2) > realtime);
- assertTrue(mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_1)
- < mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_3));
- assertTrue(mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_3)
- < mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_2));
- assertTrue(mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_1) - realtime
+ assertTrue(mUserWakeupStore.getWakeupTimeForUser(USER_ID_2) > realtime);
+ assertTrue(mUserWakeupStore.getWakeupTimeForUser(USER_ID_1)
+ < mUserWakeupStore.getWakeupTimeForUser(USER_ID_3));
+ assertTrue(mUserWakeupStore.getWakeupTimeForUser(USER_ID_3)
+ < mUserWakeupStore.getWakeupTimeForUser(USER_ID_2));
+ assertTrue(mUserWakeupStore.getWakeupTimeForUser(USER_ID_1) - realtime
< BUFFER_TIME_MS + USER_START_TIME_DEVIATION_LIMIT_MS);
- assertTrue(mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_3) - realtime
+ assertTrue(mUserWakeupStore.getWakeupTimeForUser(USER_ID_3) - realtime
< 2 * BUFFER_TIME_MS + USER_START_TIME_DEVIATION_LIMIT_MS);
- assertTrue(mUserWakeupStore.getWakeupTimeForUserForTest(USER_ID_2) - realtime
+ assertTrue(mUserWakeupStore.getWakeupTimeForUser(USER_ID_2) - realtime
< 3 * BUFFER_TIME_MS + USER_START_TIME_DEVIATION_LIMIT_MS);
}
//TODO: b/330264023 - Add tests for I/O in usersWithAlarmClocks.xml.
--
GitLab