Merge "Address sorting edge case." into sc-dev

This commit is contained in:
Kweku Adams
2021-07-27 20:42:44 +00:00
committed by Android (Google) Code Review
2 changed files with 52 additions and 20 deletions

View File

@@ -690,7 +690,6 @@ public class JobSchedulerService extends com.android.server.SystemService
@VisibleForTesting
class PendingJobComparator implements Comparator<JobStatus> {
private final SparseBooleanArray mUidHasEjCache = new SparseBooleanArray();
private final SparseLongArray mEarliestRegEnqueueTimeCache = new SparseLongArray();
/**
@@ -699,14 +698,11 @@ public class JobSchedulerService extends com.android.server.SystemService
@GuardedBy("mLock")
@VisibleForTesting
void refreshLocked() {
mUidHasEjCache.clear();
mEarliestRegEnqueueTimeCache.clear();
for (int i = 0; i < mPendingJobs.size(); ++i) {
final JobStatus job = mPendingJobs.get(i);
final int uid = job.getSourceUid();
if (job.isRequestedExpeditedJob()) {
mUidHasEjCache.put(uid, true);
} else {
if (!job.isRequestedExpeditedJob()) {
final long earliestEnqueueTime =
mEarliestRegEnqueueTimeCache.get(uid, Long.MAX_VALUE);
mEarliestRegEnqueueTimeCache.put(uid,
@@ -736,9 +732,7 @@ public class JobSchedulerService extends com.android.server.SystemService
return o1EJ ? -1 : 1;
}
}
final boolean uid1HasEj = mUidHasEjCache.get(o1.getSourceUid());
final boolean uid2HasEj = mUidHasEjCache.get(o2.getSourceUid());
if ((uid1HasEj || uid2HasEj) && (o1EJ || o2EJ)) {
if (o1EJ || o2EJ) {
// We MUST prioritize EJs ahead of regular jobs within a single app. Since we do
// that, in order to satisfy the transitivity constraint of the comparator, if
// any UID has an EJ, we must ensure that the EJ is ordered ahead of the regular
@@ -759,9 +753,13 @@ public class JobSchedulerService extends com.android.server.SystemService
} else if (uid1EarliestRegEnqueueTime > uid2EarliestRegEnqueueTime) {
return 1;
}
} else if (o1EJ && uid1EarliestRegEnqueueTime < o2.enqueueTime) {
} else if (o1EJ && uid1EarliestRegEnqueueTime <= o2.enqueueTime) {
// Include = to ensure that if we sorted an EJ ahead of a regular job at time X
// then we make sure to sort it ahead of all regular jobs at time X.
return -1;
} else if (o2EJ && uid2EarliestRegEnqueueTime < o1.enqueueTime) {
} else if (o2EJ && uid2EarliestRegEnqueueTime <= o1.enqueueTime) {
// Include = to ensure that if we sorted an EJ ahead of a regular job at time X
// then we make sure to sort it ahead of all regular jobs at time X.
return 1;
}
}

View File

@@ -885,6 +885,7 @@ public class JobSchedulerServiceTest {
// * E jobs test that expedited jobs don't skip the line when the app has no regular jobs
// * F jobs test correct expedited/regular ordering doesn't push jobs too high in list
// * G jobs test correct ordering for regular jobs
// * H job tests correct behavior when enqueue times are the same
JobStatus rA1 = createJobStatus("testPendingJobSorting", createJobInfo(1), 1);
JobStatus rB2 = createJobStatus("testPendingJobSorting", createJobInfo(2), 2);
JobStatus eC3 = createJobStatus("testPendingJobSorting",
@@ -896,6 +897,7 @@ public class JobSchedulerServiceTest {
createJobInfo(6).setExpedited(true), 2);
JobStatus eA7 = createJobStatus("testPendingJobSorting",
createJobInfo(7).setExpedited(true), 1);
JobStatus rH8 = createJobStatus("testPendingJobSorting", createJobInfo(8), 14);
JobStatus rF8 = createJobStatus("testPendingJobSorting", createJobInfo(8), 6);
JobStatus eF9 = createJobStatus("testPendingJobSorting",
createJobInfo(9).setExpedited(true), 6);
@@ -915,6 +917,7 @@ public class JobSchedulerServiceTest {
eB6.enqueueTime = 6;
eA7.enqueueTime = 7;
rF8.enqueueTime = 8;
rH8.enqueueTime = 8;
eF9.enqueueTime = 9;
rC10.enqueueTime = 10;
eC11.enqueueTime = 11;
@@ -931,6 +934,7 @@ public class JobSchedulerServiceTest {
mService.mPendingJobs.add(rD4);
mService.mPendingJobs.add(eA7);
mService.mPendingJobs.add(rG12);
mService.mPendingJobs.add(rH8);
mService.mPendingJobs.add(rF8);
mService.mPendingJobs.add(eB6);
mService.mPendingJobs.add(eE14);
@@ -943,7 +947,7 @@ public class JobSchedulerServiceTest {
mService.mPendingJobs.sort(mService.mPendingJobComparator);
final JobStatus[] expectedOrder = new JobStatus[]{
eA7, rA1, eB6, rB2, eC3, rD4, eE5, eF9, rF8, eC11, rC10, rG12, rG13, eE14};
eA7, rA1, eB6, rB2, eC3, rD4, eE5, eF9, rH8, rF8, eC11, rC10, rG12, rG13, eE14};
for (int i = 0; i < expectedOrder.length; ++i) {
assertEquals("List wasn't correctly sorted @ index " + i,
expectedOrder[i].getJobId(), mService.mPendingJobs.get(i).getJobId());
@@ -995,7 +999,7 @@ public class JobSchedulerServiceTest {
for (int i = 0; i < 2500; ++i) {
JobStatus job = createJobStatus("testPendingJobSorting_Random",
createJobInfo(i).setExpedited(random.nextBoolean()), random.nextInt(250));
job.enqueueTime = Math.abs(random.nextInt(1_000_000));
job.enqueueTime = random.nextInt(1_000_000);
mService.mPendingJobs.add(job);
mService.mPendingJobComparator.refreshLocked();
@@ -1023,18 +1027,48 @@ public class JobSchedulerServiceTest {
@Test
public void testPendingJobSortingTransitivity() {
Random random = new Random(1); // Always use the same series of pseudo random values.
// Always use the same series of pseudo random values.
for (int seed : new int[]{1337, 7357, 606, 6357, 41106010, 3, 2, 1}) {
Random random = new Random(seed);
mService.mPendingJobs.clear();
mService.mPendingJobs.clear();
for (int i = 0; i < 250; ++i) {
JobStatus job = createJobStatus("testPendingJobSortingTransitivity",
createJobInfo(i).setExpedited(random.nextBoolean()), random.nextInt(50));
job.enqueueTime = Math.abs(random.nextInt(1_000_000));
job.overrideState = random.nextInt(4);
mService.mPendingJobs.add(job);
for (int i = 0; i < 300; ++i) {
JobStatus job = createJobStatus("testPendingJobSortingTransitivity",
createJobInfo(i).setExpedited(random.nextBoolean()), random.nextInt(50));
job.enqueueTime = random.nextInt(1_000_000);
job.overrideState = random.nextInt(4);
mService.mPendingJobs.add(job);
}
verifyPendingJobComparatorTransitivity();
}
}
@Test
public void testPendingJobSortingTransitivity_Concentrated() {
// Always use the same series of pseudo random values.
for (int seed : new int[]{1337, 6000, 637739, 6357, 1, 7, 13}) {
Random random = new Random(seed);
mService.mPendingJobs.clear();
for (int i = 0; i < 300; ++i) {
JobStatus job = createJobStatus("testPendingJobSortingTransitivity_Concentrated",
createJobInfo(i).setExpedited(random.nextFloat() < .03),
random.nextInt(20));
job.enqueueTime = random.nextInt(250);
job.overrideState = random.nextFloat() < .01
? JobStatus.OVERRIDE_SORTING : JobStatus.OVERRIDE_NONE;
mService.mPendingJobs.add(job);
Log.d(TAG, sortedJobToString(job));
}
verifyPendingJobComparatorTransitivity();
}
}
private void verifyPendingJobComparatorTransitivity() {
mService.mPendingJobComparator.refreshLocked();
for (int i = 0; i < mService.mPendingJobs.size(); ++i) {