From ba827860b5ad75d70967f6bf1662fdd65291293c Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Fri, 9 Dec 2022 18:04:10 +0000 Subject: [PATCH] Give user-initiated jobs immediacy privilege for TOP app. Allow user-initiated jobs to start immediately if the app is currently in the TOP state. We also want to give that privilege when the app is in BAL state, and this refactor makes it easier to add that privilege in the future. Bug: 261999509 Test: atest FrameworksMockingServicesTests:JobConcurrencyManagerTest Change-Id: I0f5dc27522427b07f908a3b84581dbb48b524a02 --- .../server/job/JobConcurrencyManager.java | 67 ++++++++++++------- .../server/job/controllers/JobStatus.java | 2 + .../server/job/JobConcurrencyManagerTest.java | 45 +++++++++++-- 3 files changed, 81 insertions(+), 33 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java index 30986dde6b911..c90445e8877ed 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java @@ -761,8 +761,8 @@ class JobConcurrencyManager { if (js != null) { mWorkCountTracker.incrementRunningJobCount(jsc.getRunningJobWorkType()); assignment.workType = jsc.getRunningJobWorkType(); - if (js.startedAsExpeditedJob && js.lastEvaluatedBias == JobInfo.BIAS_TOP_APP) { - info.numRunningTopEj++; + if (js.startedWithImmediacyPrivilege) { + info.numRunningImmediacyPrivileged++; } } @@ -829,11 +829,9 @@ class JobConcurrencyManager { continue; } - final boolean isTopEj = nextPending.shouldTreatAsExpeditedJob() - && nextPending.lastEvaluatedBias == JobInfo.BIAS_TOP_APP; + final boolean hasImmediacyPrivilege = hasImmediacyPrivilegeLocked(nextPending); if (DEBUG && isSimilarJobRunningLocked(nextPending)) { - Slog.w(TAG, "Already running similar " + (isTopEj ? "TOP-EJ" : "job") - + " to: " + nextPending); + Slog.w(TAG, "Already running similar job to: " + nextPending); } // Factoring minChangedWaitingTimeMs into the min waiting time effectively limits @@ -876,23 +874,25 @@ class JobConcurrencyManager { final JobStatus runningJob = assignment.context.getRunningJobLocked(); // Maybe stop the job if it has had its day in the sun. Only allow replacing // for one of the following conditions: - // 1. We're putting in the current TOP app's EJ + // 1. We're putting in a job that has the privilege of running immediately // 2. There aren't too many jobs running AND the current job started when the // app was in the background // 3. There aren't too many jobs running AND the current job started when the // app was on TOP, but the app has since left TOP // 4. There aren't too many jobs running AND the current job started when the - // app was on TOP, the app is still TOP, but there are too many TOP+EJs + // app was on TOP, the app is still TOP, but there are too many + // immediacy-privileged jobs // running (because we don't want them to starve out other apps and the // current job has already run for the minimum guaranteed time). // 5. This new job could be waiting for too long for a slot to open up - boolean canReplace = isTopEj; // Case 1 + boolean canReplace = hasImmediacyPrivilege; // Case 1 if (!canReplace && !isInOverage) { final int currentJobBias = mService.evaluateJobBiasLocked(runningJob); canReplace = runningJob.lastEvaluatedBias < JobInfo.BIAS_TOP_APP // Case 2 || currentJobBias < JobInfo.BIAS_TOP_APP // Case 3 // Case 4 - || info.numRunningTopEj > .5 * mWorkTypeConfig.getMaxTotal(); + || info.numRunningImmediacyPrivileged + > (mWorkTypeConfig.getMaxTotal() / 2); } if (!canReplace && mMaxWaitTimeBypassEnabled) { // Case 5 if (nextPending.shouldTreatAsExpeditedJob()) { @@ -919,7 +919,7 @@ class JobConcurrencyManager { } } } - if (selectedContext == null && (!isInOverage || isTopEj)) { + if (selectedContext == null && (!isInOverage || hasImmediacyPrivilege)) { int lowestBiasSeen = Integer.MAX_VALUE; long newMinPreferredUidOnlyWaitingTimeMs = Long.MAX_VALUE; for (int p = preferredUidOnly.size() - 1; p >= 0; --p) { @@ -962,12 +962,13 @@ class JobConcurrencyManager { info.minPreferredUidOnlyWaitingTimeMs = newMinPreferredUidOnlyWaitingTimeMs; } } - // Make sure to run EJs for the TOP app immediately. - if (isTopEj) { + // Make sure to run jobs with special privilege immediately. + if (hasImmediacyPrivilege) { if (selectedContext != null && selectedContext.context.getRunningJobLocked() != null) { - // We're "replacing" a currently running job, but we want TOP EJs to start - // immediately, so we'll start the EJ on a fresh available context and + // We're "replacing" a currently running job, but we want immediacy-privileged + // jobs to start immediately, so we'll start the privileged jobs on a fresh + // available context and // stop this currently running job to replace in two steps. changed.add(selectedContext); projectedRunningCount--; @@ -1029,6 +1030,7 @@ class JobConcurrencyManager { projectedRunningCount--; } if (selectedContext.newJob != null) { + selectedContext.newJob.startedWithImmediacyPrivilege = hasImmediacyPrivilege; projectedRunningCount++; minChangedWaitingTimeMs = Math.min(minChangedWaitingTimeMs, mService.getMinJobExecutionGuaranteeMs(selectedContext.newJob)); @@ -1103,6 +1105,18 @@ class JobConcurrencyManager { mActivePkgStats.forEach(mPackageStatsStagingCountClearer); } + @VisibleForTesting + @GuardedBy("mLock") + boolean hasImmediacyPrivilegeLocked(@NonNull JobStatus job) { + // EJs & user-initiated jobs for the TOP app should run immediately. + // However, even for user-initiated jobs, if the app has not recently been in TOP or BAL + // state, we don't give the immediacy privilege so that we can try and maintain + // reasonably concurrency behavior. + return job.lastEvaluatedBias == JobInfo.BIAS_TOP_APP + // TODO(): include BAL state for user-initiated jobs + && (job.shouldTreatAsExpeditedJob() || job.shouldTreatAsUserInitiated()); + } + @GuardedBy("mLock") void onUidBiasChangedLocked(int prevBias, int newBias) { if (prevBias != JobInfo.BIAS_TOP_APP && newBias != JobInfo.BIAS_TOP_APP) { @@ -1361,7 +1375,7 @@ class JobConcurrencyManager { mActiveServices.remove(worker); if (mIdleContexts.size() < MAX_RETAINED_OBJECTS) { // Don't need to save all new contexts, but keep some extra around in case we need - // extras for another TOP+EJ overage. + // extras for another immediacy privileged overage. mIdleContexts.add(worker); } else { mNumDroppedContexts++; @@ -1403,7 +1417,8 @@ class JobConcurrencyManager { } if (respectConcurrencyLimit) { worker.clearPreferredUid(); - // We're over the limit (because the TOP app scheduled a lot of EJs), but we should + // We're over the limit (because there were a lot of immediacy-privileged jobs + // scheduled), but we should // be able to stop the other jobs soon so don't start running anything new until we // get back below the limit. noteConcurrency(); @@ -1627,17 +1642,17 @@ class JobConcurrencyManager { } } else if (mWorkCountTracker.getPendingJobCount(WORK_TYPE_EJ) > 0) { return "blocking " + workTypeToString(WORK_TYPE_EJ) + " queue"; - } else if (js.startedAsExpeditedJob && js.lastEvaluatedBias == JobInfo.BIAS_TOP_APP) { - // Try not to let TOP + EJ starve out other apps. - int topEjCount = 0; + } else if (js.startedWithImmediacyPrivilege) { + // Try not to let jobs with immediacy privilege starve out other apps. + int immediacyPrivilegeCount = 0; for (int r = mRunningJobs.size() - 1; r >= 0; --r) { JobStatus j = mRunningJobs.valueAt(r); - if (j.startedAsExpeditedJob && j.lastEvaluatedBias == JobInfo.BIAS_TOP_APP) { - topEjCount++; + if (j.startedWithImmediacyPrivilege) { + immediacyPrivilegeCount++; } } - if (topEjCount > .5 * mWorkTypeConfig.getMaxTotal()) { - return "prevent top EJ dominance"; + if (immediacyPrivilegeCount > mWorkTypeConfig.getMaxTotal() / 2) { + return "prevent immediacy privilege dominance"; } } // No other pending EJs. Return null so we don't let regular jobs preempt an EJ. @@ -2566,11 +2581,11 @@ class JobConcurrencyManager { @VisibleForTesting static final class AssignmentInfo { public long minPreferredUidOnlyWaitingTimeMs; - public int numRunningTopEj; + public int numRunningImmediacyPrivileged; void clear() { minPreferredUidOnlyWaitingTimeMs = 0; - numRunningTopEj = 0; + numRunningImmediacyPrivileged = 0; } } diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java index 419127e6c6a90..2e67a81f89ea6 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java @@ -388,6 +388,8 @@ public final class JobStatus { */ public boolean startedAsExpeditedJob = false; + public boolean startedWithImmediacyPrivilege = false; + // If non-null, this is work that has been enqueued for the job. public ArrayList pendingWork; diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobConcurrencyManagerTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobConcurrencyManagerTest.java index a9dc4af07bab5..480a4f358bc07 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobConcurrencyManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobConcurrencyManagerTest.java @@ -217,7 +217,7 @@ public final class JobConcurrencyManagerTest { assertEquals(0, preferredUidOnly.size()); assertEquals(0, stoppable.size()); assertEquals(0, assignmentInfo.minPreferredUidOnlyWaitingTimeMs); - assertEquals(0, assignmentInfo.numRunningTopEj); + assertEquals(0, assignmentInfo.numRunningImmediacyPrivileged); } @Test @@ -239,7 +239,7 @@ public final class JobConcurrencyManagerTest { assertEquals(0, preferredUidOnly.size()); assertEquals(0, stoppable.size()); assertEquals(0, assignmentInfo.minPreferredUidOnlyWaitingTimeMs); - assertEquals(0, assignmentInfo.numRunningTopEj); + assertEquals(0, assignmentInfo.numRunningImmediacyPrivileged); } @Test @@ -265,15 +265,14 @@ public final class JobConcurrencyManagerTest { assertEquals(JobConcurrencyManager.STANDARD_CONCURRENCY_LIMIT, preferredUidOnly.size()); assertEquals(0, stoppable.size()); assertEquals(0, assignmentInfo.minPreferredUidOnlyWaitingTimeMs); - assertEquals(0, assignmentInfo.numRunningTopEj); + assertEquals(0, assignmentInfo.numRunningImmediacyPrivileged); } @Test - public void testPrepareForAssignmentDetermination_onlyRunningTopEjs() { + public void testPrepareForAssignmentDetermination_onlyStartedWithImmediacyPrivilege() { for (int i = 0; i < JobConcurrencyManager.STANDARD_CONCURRENCY_LIMIT; ++i) { JobStatus job = createJob(mDefaultUserId * UserHandle.PER_USER_RANGE + i); - job.startedAsExpeditedJob = true; - job.lastEvaluatedBias = JobInfo.BIAS_TOP_APP; + job.startedWithImmediacyPrivilege = true; mJobConcurrencyManager.addRunningJobForTesting(job); } @@ -294,7 +293,7 @@ public final class JobConcurrencyManagerTest { assertEquals(JobConcurrencyManager.STANDARD_CONCURRENCY_LIMIT / 2, stoppable.size()); assertEquals(0, assignmentInfo.minPreferredUidOnlyWaitingTimeMs); assertEquals(JobConcurrencyManager.STANDARD_CONCURRENCY_LIMIT, - assignmentInfo.numRunningTopEj); + assignmentInfo.numRunningImmediacyPrivileged); } @Test @@ -499,6 +498,38 @@ public final class JobConcurrencyManagerTest { } } + @Test + public void testHasImmediacyPrivilege() { + JobStatus job = createJob(mDefaultUserId * UserHandle.PER_USER_RANGE, 0); + spyOn(job); + assertFalse(mJobConcurrencyManager.hasImmediacyPrivilegeLocked(job)); + + doReturn(false).when(job).shouldTreatAsExpeditedJob(); + doReturn(false).when(job).shouldTreatAsUserInitiated(); + job.lastEvaluatedBias = JobInfo.BIAS_TOP_APP; + assertFalse(mJobConcurrencyManager.hasImmediacyPrivilegeLocked(job)); + + doReturn(true).when(job).shouldTreatAsExpeditedJob(); + doReturn(false).when(job).shouldTreatAsUserInitiated(); + job.lastEvaluatedBias = JobInfo.BIAS_DEFAULT; + assertFalse(mJobConcurrencyManager.hasImmediacyPrivilegeLocked(job)); + + doReturn(false).when(job).shouldTreatAsExpeditedJob(); + doReturn(true).when(job).shouldTreatAsUserInitiated(); + job.lastEvaluatedBias = JobInfo.BIAS_DEFAULT; + assertFalse(mJobConcurrencyManager.hasImmediacyPrivilegeLocked(job)); + + doReturn(false).when(job).shouldTreatAsExpeditedJob(); + doReturn(true).when(job).shouldTreatAsUserInitiated(); + job.lastEvaluatedBias = JobInfo.BIAS_TOP_APP; + assertTrue(mJobConcurrencyManager.hasImmediacyPrivilegeLocked(job)); + + doReturn(true).when(job).shouldTreatAsExpeditedJob(); + doReturn(false).when(job).shouldTreatAsUserInitiated(); + job.lastEvaluatedBias = JobInfo.BIAS_TOP_APP; + assertTrue(mJobConcurrencyManager.hasImmediacyPrivilegeLocked(job)); + } + @Test public void testIsPkgConcurrencyLimited_top() { final JobStatus topJob = createJob(mDefaultUserId * UserHandle.PER_USER_RANGE, 0);