Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source
This commit is contained in:
Kweku Adams
2021-07-27 20:42:44 +00:00
committed by Android (Google) Code Review
parent 06a68d9a72 0687577f87
commit ab3f902ccf
2 changed files with 52 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
View File

@@ -690,7 +690,6 @@ public class JobSchedulerService extends com.android.server.SystemService
@VisibleForTesting @VisibleForTesting
class PendingJobComparator implements Comparator<JobStatus> { class PendingJobComparator implements Comparator<JobStatus> {
private final SparseBooleanArray mUidHasEjCache = new SparseBooleanArray();
private final SparseLongArray mEarliestRegEnqueueTimeCache = new SparseLongArray(); private final SparseLongArray mEarliestRegEnqueueTimeCache = new SparseLongArray();
/** /**
@@ -699,14 +698,11 @@ public class JobSchedulerService extends com.android.server.SystemService
@GuardedBy("mLock") @GuardedBy("mLock")
@VisibleForTesting @VisibleForTesting
void refreshLocked() { void refreshLocked() {
mUidHasEjCache.clear();
mEarliestRegEnqueueTimeCache.clear(); mEarliestRegEnqueueTimeCache.clear();
for (int i = 0; i < mPendingJobs.size(); ++i) { for (int i = 0; i < mPendingJobs.size(); ++i) {
final JobStatus job = mPendingJobs.get(i); final JobStatus job = mPendingJobs.get(i);
final int uid = job.getSourceUid(); final int uid = job.getSourceUid();
if (job.isRequestedExpeditedJob()) { if (!job.isRequestedExpeditedJob()) {
mUidHasEjCache.put(uid, true);
} else {
final long earliestEnqueueTime = final long earliestEnqueueTime =
mEarliestRegEnqueueTimeCache.get(uid, Long.MAX_VALUE); mEarliestRegEnqueueTimeCache.get(uid, Long.MAX_VALUE);
mEarliestRegEnqueueTimeCache.put(uid, mEarliestRegEnqueueTimeCache.put(uid,
@@ -736,9 +732,7 @@ public class JobSchedulerService extends com.android.server.SystemService
return o1EJ ? -1 : 1; return o1EJ ? -1 : 1;
} }
} }
final boolean uid1HasEj = mUidHasEjCache.get(o1.getSourceUid()); if (o1EJ || o2EJ) {
final boolean uid2HasEj = mUidHasEjCache.get(o2.getSourceUid());
if ((uid1HasEj || uid2HasEj) && (o1EJ || o2EJ)) {
// We MUST prioritize EJs ahead of regular jobs within a single app. Since we do // 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 // 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 // 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) { } else if (uid1EarliestRegEnqueueTime > uid2EarliestRegEnqueueTime) {
return 1; 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; 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; return 1;
} }
} }

54
services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java
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 // * 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 // * F jobs test correct expedited/regular ordering doesn't push jobs too high in list
// * G jobs test correct ordering for regular jobs // * 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 rA1 = createJobStatus("testPendingJobSorting", createJobInfo(1), 1);
JobStatus rB2 = createJobStatus("testPendingJobSorting", createJobInfo(2), 2); JobStatus rB2 = createJobStatus("testPendingJobSorting", createJobInfo(2), 2);
JobStatus eC3 = createJobStatus("testPendingJobSorting", JobStatus eC3 = createJobStatus("testPendingJobSorting",
@@ -896,6 +897,7 @@ public class JobSchedulerServiceTest {
createJobInfo(6).setExpedited(true), 2); createJobInfo(6).setExpedited(true), 2);
JobStatus eA7 = createJobStatus("testPendingJobSorting", JobStatus eA7 = createJobStatus("testPendingJobSorting",
createJobInfo(7).setExpedited(true), 1); createJobInfo(7).setExpedited(true), 1);
JobStatus rH8 = createJobStatus("testPendingJobSorting", createJobInfo(8), 14);
JobStatus rF8 = createJobStatus("testPendingJobSorting", createJobInfo(8), 6); JobStatus rF8 = createJobStatus("testPendingJobSorting", createJobInfo(8), 6);
JobStatus eF9 = createJobStatus("testPendingJobSorting", JobStatus eF9 = createJobStatus("testPendingJobSorting",
createJobInfo(9).setExpedited(true), 6); createJobInfo(9).setExpedited(true), 6);
@@ -915,6 +917,7 @@ public class JobSchedulerServiceTest {
eB6.enqueueTime = 6; eB6.enqueueTime = 6;
eA7.enqueueTime = 7; eA7.enqueueTime = 7;
rF8.enqueueTime = 8; rF8.enqueueTime = 8;
rH8.enqueueTime = 8;
eF9.enqueueTime = 9; eF9.enqueueTime = 9;
rC10.enqueueTime = 10; rC10.enqueueTime = 10;
eC11.enqueueTime = 11; eC11.enqueueTime = 11;
@@ -931,6 +934,7 @@ public class JobSchedulerServiceTest {
mService.mPendingJobs.add(rD4); mService.mPendingJobs.add(rD4);
mService.mPendingJobs.add(eA7); mService.mPendingJobs.add(eA7);
mService.mPendingJobs.add(rG12); mService.mPendingJobs.add(rG12);
mService.mPendingJobs.add(rH8);
mService.mPendingJobs.add(rF8); mService.mPendingJobs.add(rF8);
mService.mPendingJobs.add(eB6); mService.mPendingJobs.add(eB6);
mService.mPendingJobs.add(eE14); mService.mPendingJobs.add(eE14);
@@ -943,7 +947,7 @@ public class JobSchedulerServiceTest {
mService.mPendingJobs.sort(mService.mPendingJobComparator); mService.mPendingJobs.sort(mService.mPendingJobComparator);
final JobStatus[] expectedOrder = new JobStatus[]{ 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) { for (int i = 0; i < expectedOrder.length; ++i) {
assertEquals("List wasn't correctly sorted @ index " + i, assertEquals("List wasn't correctly sorted @ index " + i,
expectedOrder[i].getJobId(), mService.mPendingJobs.get(i).getJobId()); expectedOrder[i].getJobId(), mService.mPendingJobs.get(i).getJobId());
@@ -995,7 +999,7 @@ public class JobSchedulerServiceTest {
for (int i = 0; i < 2500; ++i) { for (int i = 0; i < 2500; ++i) {
JobStatus job = createJobStatus("testPendingJobSorting_Random", JobStatus job = createJobStatus("testPendingJobSorting_Random",
createJobInfo(i).setExpedited(random.nextBoolean()), random.nextInt(250)); 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.mPendingJobs.add(job);
mService.mPendingJobComparator.refreshLocked(); mService.mPendingJobComparator.refreshLocked();
@@ -1023,18 +1027,48 @@ public class JobSchedulerServiceTest {
@Test @Test
public void testPendingJobSortingTransitivity() { 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) { for (int i = 0; i < 300; ++i) {
JobStatus job = createJobStatus("testPendingJobSortingTransitivity", JobStatus job = createJobStatus("testPendingJobSortingTransitivity",
createJobInfo(i).setExpedited(random.nextBoolean()), random.nextInt(50)); createJobInfo(i).setExpedited(random.nextBoolean()), random.nextInt(50));
job.enqueueTime = Math.abs(random.nextInt(1_000_000)); job.enqueueTime = random.nextInt(1_000_000);
job.overrideState = random.nextInt(4); job.overrideState = random.nextInt(4);
mService.mPendingJobs.add(job); 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(); mService.mPendingJobComparator.refreshLocked();
for (int i = 0; i < mService.mPendingJobs.size(); ++i) { for (int i = 0; i < mService.mPendingJobs.size(); ++i) {