Merge "Try to avoid excessive delays for UI jobs."

This commit is contained in:
TreeHugger Robot
2023-02-10 17:48:35 +00:00
committed by Android (Google) Code Review
2 changed files with 186 additions and 30 deletions

View File

@@ -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;
}
}

View File

@@ -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<JobStatus> 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<JobStatus> 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<JobConcurrencyManager.ContextAssignment> changed = new ArraySet<>();
final ArraySet<JobConcurrencyManager.ContextAssignment> idle = new ArraySet<>();
final List<JobConcurrencyManager.ContextAssignment> preferredUidOnly = new ArrayList<>();
final List<JobConcurrencyManager.ContextAssignment> 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);
}
}