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 a474cc836f2c5..1022490197439 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java @@ -129,6 +129,10 @@ class JobConcurrencyManager { static final String KEY_ENABLE_MAX_WAIT_TIME_BYPASS = CONFIG_KEY_PREFIX_CONCURRENCY + "enable_max_wait_time_bypass"; private static final boolean DEFAULT_ENABLE_MAX_WAIT_TIME_BYPASS = true; + @VisibleForTesting + static final String KEY_MAX_WAIT_UI_MS = CONFIG_KEY_PREFIX_CONCURRENCY + "max_wait_ui_ms"; + @VisibleForTesting + static final long DEFAULT_MAX_WAIT_UI_MS = 5 * MINUTE_IN_MILLIS; private static final String KEY_MAX_WAIT_EJ_MS = CONFIG_KEY_PREFIX_CONCURRENCY + "max_wait_ej_ms"; @VisibleForTesting @@ -432,6 +436,12 @@ class JobConcurrencyManager { private boolean mMaxWaitTimeBypassEnabled = DEFAULT_ENABLE_MAX_WAIT_TIME_BYPASS; + /** + * The maximum time a user-initiated job would have to be potentially waiting for an available + * slot before we would consider creating a new slot for it. + */ + private long mMaxWaitUIMs = DEFAULT_MAX_WAIT_UI_MS; + /** * The maximum time an expedited job would have to be potentially waiting for an available * slot before we would consider creating a new slot for it. @@ -825,6 +835,13 @@ class JobConcurrencyManager { if (js.startedWithImmediacyPrivilege) { info.numRunningImmediacyPrivileged++; } + if (js.shouldTreatAsUserInitiatedJob()) { + info.numRunningUi++; + } else if (js.startedAsExpeditedJob) { + info.numRunningEj++; + } else { + info.numRunningReg++; + } } assignment.preferredUid = jsc.getPreferredUid(); @@ -880,6 +897,13 @@ class JobConcurrencyManager { JobStatus nextPending; int projectedRunningCount = activeServices.size(); long minChangedWaitingTimeMs = Long.MAX_VALUE; + // Only allow the Context creation bypass for each type if one of that type isn't already + // running. That way, we don't run into issues (creating too many additional contexts) + // if new jobs become ready to run in rapid succession and we end up going through this + // loop many times before running jobs have had a decent chance to finish. + boolean allowMaxWaitContextBypassUi = info.numRunningUi == 0; + boolean allowMaxWaitContextBypassEj = info.numRunningEj == 0; + boolean allowMaxWaitContextBypassOthers = info.numRunningReg == 0; while ((nextPending = pendingJobQueue.next()) != null) { if (mRunningJobs.contains(nextPending)) { // Should never happen. @@ -957,7 +981,9 @@ class JobConcurrencyManager { > (mWorkTypeConfig.getMaxTotal() / 2); } if (!canReplace && mMaxWaitTimeBypassEnabled) { // Case 5 - if (nextPending.shouldTreatAsExpeditedJob()) { + if (nextPending.shouldTreatAsUserInitiatedJob()) { + canReplace = minWaitingTimeMs >= mMaxWaitUIMs; + } else if (nextPending.shouldTreatAsExpeditedJob()) { canReplace = minWaitingTimeMs >= mMaxWaitEjMs; } else { canReplace = minWaitingTimeMs >= mMaxWaitRegularMs; @@ -1055,9 +1081,25 @@ class JobConcurrencyManager { (workType != WORK_TYPE_NONE) ? workType : WORK_TYPE_TOP; } } else if (selectedContext == null && mMaxWaitTimeBypassEnabled) { - final boolean wouldBeWaitingTooLong = nextPending.shouldTreatAsExpeditedJob() - ? minWaitingTimeMs >= mMaxWaitEjMs - : minWaitingTimeMs >= mMaxWaitRegularMs; + final boolean wouldBeWaitingTooLong; + if (nextPending.shouldTreatAsUserInitiatedJob() && allowMaxWaitContextBypassUi) { + wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitUIMs; + // We want to create at most one additional context for each type. + allowMaxWaitContextBypassUi = !wouldBeWaitingTooLong; + } else if (nextPending.shouldTreatAsExpeditedJob() && allowMaxWaitContextBypassEj) { + wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitEjMs; + // We want to create at most one additional context for each type. + allowMaxWaitContextBypassEj = !wouldBeWaitingTooLong; + } else if (allowMaxWaitContextBypassOthers) { + // The way things are set up a UIJ or EJ could end up here and create a 2nd + // context as if it were a "regular" job. That's fine for now since they would + // still be subject to the higher waiting time threshold here. + wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitRegularMs; + // We want to create at most one additional context for each type. + allowMaxWaitContextBypassOthers = !wouldBeWaitingTooLong; + } else { + wouldBeWaitingTooLong = false; + } if (wouldBeWaitingTooLong) { if (DEBUG) { Slog.d(TAG, "Allowing additional context because job would wait too long"); @@ -1493,10 +1535,14 @@ class JobConcurrencyManager { minWaitingTimeMs = Math.min(minWaitingTimeMs, mActiveServices.get(i).getRemainingGuaranteedTimeMs(nowElapsed)); } - final boolean wouldBeWaitingTooLong = - mWorkCountTracker.getPendingJobCount(WORK_TYPE_EJ) > 0 - ? minWaitingTimeMs >= mMaxWaitEjMs - : minWaitingTimeMs >= mMaxWaitRegularMs; + final boolean wouldBeWaitingTooLong; + if (mWorkCountTracker.getPendingJobCount(WORK_TYPE_UI) > 0) { + wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitUIMs; + } else if (mWorkCountTracker.getPendingJobCount(WORK_TYPE_EJ) > 0) { + wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitEjMs; + } else { + wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitRegularMs; + } respectConcurrencyLimit = !wouldBeWaitingTooLong; } if (respectConcurrencyLimit) { @@ -1911,8 +1957,11 @@ class JobConcurrencyManager { mMaxWaitTimeBypassEnabled = properties.getBoolean( KEY_ENABLE_MAX_WAIT_TIME_BYPASS, DEFAULT_ENABLE_MAX_WAIT_TIME_BYPASS); - // EJ max wait must be in the range [0, infinity). - mMaxWaitEjMs = Math.max(0, properties.getLong(KEY_MAX_WAIT_EJ_MS, DEFAULT_MAX_WAIT_EJ_MS)); + // UI max wait must be in the range [0, infinity). + mMaxWaitUIMs = Math.max(0, properties.getLong(KEY_MAX_WAIT_UI_MS, DEFAULT_MAX_WAIT_UI_MS)); + // EJ max wait must be in the range [UI max wait, infinity). + mMaxWaitEjMs = Math.max(mMaxWaitUIMs, + properties.getLong(KEY_MAX_WAIT_EJ_MS, DEFAULT_MAX_WAIT_EJ_MS)); // Regular max wait must be in the range [EJ max wait, infinity). mMaxWaitRegularMs = Math.max(mMaxWaitEjMs, properties.getLong(KEY_MAX_WAIT_REGULAR_MS, DEFAULT_MAX_WAIT_REGULAR_MS)); @@ -1931,6 +1980,7 @@ class JobConcurrencyManager { pw.print(KEY_PKG_CONCURRENCY_LIMIT_EJ, mPkgConcurrencyLimitEj).println(); pw.print(KEY_PKG_CONCURRENCY_LIMIT_REGULAR, mPkgConcurrencyLimitRegular).println(); pw.print(KEY_ENABLE_MAX_WAIT_TIME_BYPASS, mMaxWaitTimeBypassEnabled).println(); + pw.print(KEY_MAX_WAIT_UI_MS, mMaxWaitUIMs).println(); pw.print(KEY_MAX_WAIT_EJ_MS, mMaxWaitEjMs).println(); pw.print(KEY_MAX_WAIT_REGULAR_MS, mMaxWaitRegularMs).println(); pw.println(); @@ -2793,10 +2843,16 @@ class JobConcurrencyManager { static final class AssignmentInfo { public long minPreferredUidOnlyWaitingTimeMs; public int numRunningImmediacyPrivileged; + public int numRunningUi; + public int numRunningEj; + public int numRunningReg; void clear() { minPreferredUidOnlyWaitingTimeMs = 0; numRunningImmediacyPrivileged = 0; + numRunningUi = 0; + numRunningEj = 0; + numRunningReg = 0; } } 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 ddb6f239a2c3c..3750aa0f7c011 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobConcurrencyManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobConcurrencyManagerTest.java @@ -335,13 +335,14 @@ public final class JobConcurrencyManagerTest { mConfigBuilder.setBoolean(JobConcurrencyManager.KEY_ENABLE_MAX_WAIT_TIME_BYPASS, true); setConcurrencyConfig(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, new TypeConfig(WORK_TYPE_BG, 0, JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT)); - for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 2; ++i) { + for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 3; ++i) { final int uid = mDefaultUserId * UserHandle.PER_USER_RANGE + i; final String sourcePkgName = "com.source.package." + UserHandle.getAppId(uid); setPackageUid(sourcePkgName, uid); final JobStatus job = createJob(uid, sourcePkgName); spyOn(job); - doReturn(i % 2 == 0).when(job).shouldTreatAsExpeditedJob(); + doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob(); + doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob(); if (i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) { mJobConcurrencyManager.addRunningJobForTesting(job); } else { @@ -350,7 +351,7 @@ public final class JobConcurrencyManagerTest { } // Waiting time is too short, so we shouldn't create any extra contexts. - final long remainingTimeMs = JobConcurrencyManager.DEFAULT_MAX_WAIT_EJ_MS / 2; + final long remainingTimeMs = JobConcurrencyManager.DEFAULT_MAX_WAIT_UI_MS / 2; for (int i = 0; i < mInjector.contexts.size(); ++i) { doReturn(true).when(mInjector.contexts.keyAt(i)).isWithinExecutionGuaranteeTime(); doReturn(remainingTimeMs) @@ -378,19 +379,25 @@ public final class JobConcurrencyManagerTest { } @Test - public void testDetermineAssignments_allPreferredUidOnly_mediumTimeLeft() throws Exception { + public void testDetermineAssignments_allPreferredUidOnly_mediumTimeLeft_onlyRegRunning() + throws Exception { mConfigBuilder.setBoolean(JobConcurrencyManager.KEY_ENABLE_MAX_WAIT_TIME_BYPASS, true); + // Set the waiting time to be less than an EJ's min execution time. + mConfigBuilder.setLong(JobConcurrencyManager.KEY_MAX_WAIT_UI_MS, 2 * 60_000L); setConcurrencyConfig(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, new TypeConfig(WORK_TYPE_BG, 0, JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT)); final ArraySet jobs = new ArraySet<>(); - for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 2; ++i) { + for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 4; ++i) { final int uid = mDefaultUserId * UserHandle.PER_USER_RANGE + i; final String sourcePkgName = "com.source.package." + UserHandle.getAppId(uid); setPackageUid(sourcePkgName, uid); final JobStatus job = createJob(uid, sourcePkgName); spyOn(job); - doReturn(i % 2 == 0).when(job).shouldTreatAsExpeditedJob(); - if (i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) { + doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob(); + doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob(); + if (i % 3 == 2 + && mJobConcurrencyManager.mActiveServices.size() + < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) { mJobConcurrencyManager.addRunningJobForTesting(job); } else { mPendingJobQueue.add(job); @@ -398,8 +405,8 @@ public final class JobConcurrencyManagerTest { } } - // Waiting time is longer than the EJ waiting time, but shorter than regular job waiting - // time, so we should only create an extra context for an EJ. + // Waiting time is longer than the EJ & UI waiting time, but shorter than regular job + // waiting time, so we should only create 2 extra contexts (one for EJ, one for UIJ). final long remainingTimeMs = (JobConcurrencyManager.DEFAULT_MAX_WAIT_EJ_MS + JobConcurrencyManager.DEFAULT_MAX_WAIT_REGULAR_MS) / 2; for (int i = 0; i < mInjector.contexts.size(); ++i) { @@ -428,7 +435,81 @@ public final class JobConcurrencyManagerTest { for (int i = changed.size() - 1; i >= 0; --i) { jobs.remove(changed.valueAt(i).newJob); } - assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - 1, jobs.size()); + // 1 EJ & 1 UIJ removed from the pending list. + assertEquals(3 * JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - 2, jobs.size()); + assertEquals(2, changed.size()); + JobStatus assignedJob1 = changed.valueAt(0).newJob; + JobStatus assignedJob2 = changed.valueAt(1).newJob; + boolean ejFirst = assignedJob1.shouldTreatAsExpeditedJob(); + if (ejFirst) { + assertTrue(assignedJob1.shouldTreatAsExpeditedJob()); + assertTrue(assignedJob2.shouldTreatAsUserInitiatedJob()); + } else { + assertTrue(assignedJob1.shouldTreatAsUserInitiatedJob()); + assertTrue(assignedJob2.shouldTreatAsExpeditedJob()); + } + } + + @Test + public void testDetermineAssignments_allPreferredUidOnly_mediumTimeLeft_onlyUiRunning() + throws Exception { + mConfigBuilder.setBoolean(JobConcurrencyManager.KEY_ENABLE_MAX_WAIT_TIME_BYPASS, true); + // Set the waiting time to be less than an EJ's min execution time. + mConfigBuilder.setLong(JobConcurrencyManager.KEY_MAX_WAIT_UI_MS, 2 * 60_000L); + setConcurrencyConfig(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, + new TypeConfig(WORK_TYPE_BG, 0, JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT)); + final ArraySet jobs = new ArraySet<>(); + for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 4; ++i) { + final int uid = mDefaultUserId * UserHandle.PER_USER_RANGE + i; + final String sourcePkgName = "com.source.package." + UserHandle.getAppId(uid); + setPackageUid(sourcePkgName, uid); + final JobStatus job = createJob(uid, sourcePkgName); + spyOn(job); + doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob(); + doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob(); + if (i % 3 == 0 + && mJobConcurrencyManager.mActiveServices.size() + < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) { + mJobConcurrencyManager.addRunningJobForTesting(job); + } else { + mPendingJobQueue.add(job); + jobs.add(job); + } + } + + // Waiting time is longer than the EJ & UI waiting time, but shorter than regular job + // waiting time, so we should only create 2 extra contexts (one for EJ, one for UIJ). + final long remainingTimeMs = (JobConcurrencyManager.DEFAULT_MAX_WAIT_EJ_MS + + JobConcurrencyManager.DEFAULT_MAX_WAIT_REGULAR_MS) / 2; + for (int i = 0; i < mInjector.contexts.size(); ++i) { + doReturn(true).when(mInjector.contexts.keyAt(i)).isWithinExecutionGuaranteeTime(); + doReturn(remainingTimeMs) + .when(mInjector.contexts.keyAt(i)).getRemainingGuaranteedTimeMs(anyLong()); + } + + final ArraySet changed = new ArraySet<>(); + final ArraySet idle = new ArraySet<>(); + final List preferredUidOnly = new ArrayList<>(); + final List stoppable = new ArrayList<>(); + final JobConcurrencyManager.AssignmentInfo assignmentInfo = + new JobConcurrencyManager.AssignmentInfo(); + + mJobConcurrencyManager.prepareForAssignmentDeterminationLocked( + idle, preferredUidOnly, stoppable, assignmentInfo); + assertEquals(remainingTimeMs, assignmentInfo.minPreferredUidOnlyWaitingTimeMs); + assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, preferredUidOnly.size()); + + mJobConcurrencyManager + .determineAssignmentsLocked(changed, idle, preferredUidOnly, stoppable, + assignmentInfo); + + assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, preferredUidOnly.size()); + for (int i = changed.size() - 1; i >= 0; --i) { + jobs.remove(changed.valueAt(i).newJob); + } + // There are already UIJs running, and wait time is too long for regular jobs, so + // only 1 EJ removed from the pending list. + assertEquals(3 * JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - 1, jobs.size()); assertEquals(1, changed.size()); JobStatus assignedJob = changed.valueAt(0).newJob; assertTrue(assignedJob.shouldTreatAsExpeditedJob()); @@ -446,7 +527,8 @@ public final class JobConcurrencyManagerTest { setPackageUid(sourcePkgName, uid); final JobStatus job = createJob(uid, sourcePkgName); spyOn(job); - doReturn(i % 2 == 0).when(job).shouldTreatAsExpeditedJob(); + doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob(); + doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob(); if (i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) { mJobConcurrencyManager.addRunningJobForTesting(job); } else { @@ -481,21 +563,39 @@ public final class JobConcurrencyManagerTest { assignmentInfo); assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, preferredUidOnly.size()); - // Depending on iteration order, we may create 1 or 2 contexts. + // Depending on iteration order, we may create 1-3 contexts. final long numAssignedJobs = changed.size(); assertTrue(numAssignedJobs > 0); - assertTrue(numAssignedJobs <= 2); + assertTrue(numAssignedJobs <= 3); + int numUi = 0, numEj = 0, numReg = 0; for (int i = 0; i < numAssignedJobs; ++i) { - jobs.remove(changed.valueAt(i).newJob); + JobStatus assignedJob = changed.valueAt(i).newJob; + jobs.remove(assignedJob); + if (assignedJob.shouldTreatAsUserInitiatedJob()) { + numUi++; + } else if (assignedJob.shouldTreatAsExpeditedJob()) { + numEj++; + } else { + numReg++; + } } assertEquals(numAssignedJobs, JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - jobs.size()); - JobStatus firstAssignedJob = changed.valueAt(0).newJob; - if (!firstAssignedJob.shouldTreatAsExpeditedJob()) { - assertEquals(2, numAssignedJobs); - assertTrue(changed.valueAt(1).newJob.shouldTreatAsExpeditedJob()); - } else if (numAssignedJobs == 2) { - assertFalse(changed.valueAt(1).newJob.shouldTreatAsExpeditedJob()); + if (numReg > 0) { + assertEquals(1, numReg); + assertEquals(1, numEj); + assertEquals(1, numUi); + assertEquals(3, numAssignedJobs); + } else { + if (numEj > 0) { + assertEquals(1, numEj); + } + // If the manager looks at an EJ before a UIJ, the waiting time for the UIJ will drop + // to 3 minutes and be below the threshold to create a new context. + if (numUi > 0) { + assertEquals(1, numUi); + } + assertEquals(numEj + numUi, numAssignedJobs); } }