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

Merge "Fix pending job sorting." into sc-dev

Browse Source
This commit is contained in:
Kweku Adams
2021-06-28 16:29:42 +00:00
committed by Android (Google) Code Review
parent e665d39fd4 2700aba1c5
commit 881da35724
2 changed files with 326 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -73,7 +73,9 @@ import android.util.IndentingPrintWriter;
import android.util.Log;
import android.util.Slog;
import android.util.SparseArray;
import android.util.SparseBooleanArray;
import android.util.SparseIntArray;
import android.util.SparseLongArray;
import android.util.SparseSetArray;
import android.util.TimeUtils;
import android.util.proto.ProtoOutputStream;
@@ -686,26 +688,92 @@ public class JobSchedulerService extends com.android.server.SystemService
final Constants mConstants;
final ConstantsObserver mConstantsObserver;
private static final Comparator<JobStatus> sPendingJobComparator = (o1, o2) -> {
// Jobs with an override state set (via adb) should be put first as tests/developers
// expect the jobs to run immediately.
if (o1.overrideState != o2.overrideState) {
// Higher override state (OVERRIDE_FULL) should be before lower state (OVERRIDE_SOFT)
return o2.overrideState - o1.overrideState;
}
if (o1.getSourceUid() == o2.getSourceUid()) {
final boolean o1FGJ = o1.isRequestedExpeditedJob();
if (o1FGJ != o2.isRequestedExpeditedJob()) {
// Attempt to run requested expedited jobs ahead of regular jobs, regardless of
// expedited job quota.
return o1FGJ ? -1 : 1;
@VisibleForTesting
class PendingJobComparator implements Comparator<JobStatus> {
private final SparseBooleanArray mUidHasEjCache = new SparseBooleanArray();
private final SparseLongArray mEarliestRegEnqueueTimeCache = new SparseLongArray();
/**
* Refresh sorting determinants based on the current state of {@link #mPendingJobs}.
*/
@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 {
final long earliestEnqueueTime =
mEarliestRegEnqueueTimeCache.get(uid, Long.MAX_VALUE);
mEarliestRegEnqueueTimeCache.put(uid,
Math.min(earliestEnqueueTime, job.enqueueTime));
}
}
}
if (o1.enqueueTime < o2.enqueueTime) {
return -1;
@Override
public int compare(JobStatus o1, JobStatus o2) {
if (o1 == o2) {
return 0;
}
// Jobs with an override state set (via adb) should be put first as tests/developers
// expect the jobs to run immediately.
if (o1.overrideState != o2.overrideState) {
// Higher override state (OVERRIDE_FULL) should be before lower state
// (OVERRIDE_SOFT)
return o2.overrideState - o1.overrideState;
}
final boolean o1EJ = o1.isRequestedExpeditedJob();
final boolean o2EJ = o2.isRequestedExpeditedJob();
if (o1.getSourceUid() == o2.getSourceUid()) {
if (o1EJ != o2EJ) {
// Attempt to run requested expedited jobs ahead of regular jobs, regardless of
// expedited job quota.
return o1EJ ? -1 : 1;
}
}
final boolean uid1HasEj = mUidHasEjCache.get(o1.getSourceUid());
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
// 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
// job of a different app IF the app with an EJ had another job that came before
// the differing app. For example, if app A has regJob1 at t1 and eJob3 at t3 and
// app B has regJob2 at t2, eJob3 must be ordered before regJob2 because it will be
// ordered before regJob1.
// Regular jobs don't need to jump the line.
final long uid1EarliestRegEnqueueTime = Math.min(o1.enqueueTime,
mEarliestRegEnqueueTimeCache.get(o1.getSourceUid(), Long.MAX_VALUE));
final long uid2EarliestRegEnqueueTime = Math.min(o2.enqueueTime,
mEarliestRegEnqueueTimeCache.get(o2.getSourceUid(), Long.MAX_VALUE));
if (o1EJ && o2EJ) {
if (uid1EarliestRegEnqueueTime < uid2EarliestRegEnqueueTime) {
return -1;
} else if (uid1EarliestRegEnqueueTime > uid2EarliestRegEnqueueTime) {
return 1;
}
} else if (o1EJ && uid1EarliestRegEnqueueTime < o2.enqueueTime) {
return -1;
} else if (o2EJ && uid2EarliestRegEnqueueTime < o1.enqueueTime) {
return 1;
}
}
if (o1.enqueueTime < o2.enqueueTime) {
return -1;
}
return o1.enqueueTime > o2.enqueueTime ? 1 : 0;
}
return o1.enqueueTime > o2.enqueueTime ? 1 : 0;
};
}
@VisibleForTesting
final PendingJobComparator mPendingJobComparator = new PendingJobComparator();
static <T> void addOrderedItem(ArrayList<T> array, T newItem, Comparator<T> comparator) {
int where = Collections.binarySearch(array, newItem, comparator);
@@ -1115,7 +1183,7 @@ public class JobSchedulerService extends com.android.server.SystemService
// This is a new job, we can just immediately put it on the pending
// list and try to run it.
mJobPackageTracker.notePending(jobStatus);
addOrderedItem(mPendingJobs, jobStatus, sPendingJobComparator);
addOrderedItem(mPendingJobs, jobStatus, mPendingJobComparator);
maybeRunPendingJobsLocked();
} else {
evaluateControllerStatesLocked(jobStatus);
@@ -1919,7 +1987,7 @@ public class JobSchedulerService extends com.android.server.SystemService
if (js != null) {
if (isReadyToBeExecutedLocked(js)) {
mJobPackageTracker.notePending(js);
addOrderedItem(mPendingJobs, js, sPendingJobComparator);
addOrderedItem(mPendingJobs, js, mPendingJobComparator);
}
} else {
Slog.e(TAG, "Given null job to check individually");
@@ -2064,6 +2132,7 @@ public class JobSchedulerService extends com.android.server.SystemService
* Run through list of jobs and execute all possible - at least one is expired so we do
* as many as we can.
*/
@GuardedBy("mLock")
private void queueReadyJobsForExecutionLocked() {
// This method will check and capture all ready jobs, so we don't need to keep any messages
// in the queue.
@@ -2079,7 +2148,7 @@ public class JobSchedulerService extends com.android.server.SystemService
mPendingJobs.clear();
stopNonReadyActiveJobsLocked();
mJobs.forEachJob(mReadyQueueFunctor);
mReadyQueueFunctor.postProcess();
mReadyQueueFunctor.postProcessLocked();
if (DEBUG) {
final int queuedJobs = mPendingJobs.size();
@@ -2106,16 +2175,19 @@ public class JobSchedulerService extends com.android.server.SystemService
}
}
public void postProcess() {
@GuardedBy("mLock")
private void postProcessLocked() {
noteJobsPending(newReadyJobs);
mPendingJobs.addAll(newReadyJobs);
if (mPendingJobs.size() > 1) {
mPendingJobs.sort(sPendingJobComparator);
mPendingJobComparator.refreshLocked();
mPendingJobs.sort(mPendingJobComparator);
}
newReadyJobs.clear();
}
}
private final ReadyJobQueueFunctor mReadyQueueFunctor = new ReadyJobQueueFunctor();
/**
@@ -2180,7 +2252,9 @@ public class JobSchedulerService extends com.android.server.SystemService
}
}
public void postProcess() {
@GuardedBy("mLock")
@VisibleForTesting
void postProcessLocked() {
if (unbatchedCount > 0
|| forceBatchedCount >= mConstants.MIN_READY_NON_ACTIVE_JOBS_COUNT) {
if (DEBUG) {
@@ -2189,7 +2263,8 @@ public class JobSchedulerService extends com.android.server.SystemService
noteJobsPending(runnableJobs);
mPendingJobs.addAll(runnableJobs);
if (mPendingJobs.size() > 1) {
mPendingJobs.sort(sPendingJobComparator);
mPendingJobComparator.refreshLocked();
mPendingJobs.sort(mPendingJobComparator);
}
} else {
if (DEBUG) {
@@ -2210,6 +2285,7 @@ public class JobSchedulerService extends com.android.server.SystemService
}
private final MaybeReadyJobQueueFunctor mMaybeQueueFunctor = new MaybeReadyJobQueueFunctor();
@GuardedBy("mLock")
private void maybeQueueReadyJobsForExecutionLocked() {
if (DEBUG) Slog.d(TAG, "Maybe queuing ready jobs...");
@@ -2217,7 +2293,7 @@ public class JobSchedulerService extends com.android.server.SystemService
mPendingJobs.clear();
stopNonReadyActiveJobsLocked();
mJobs.forEachJob(mMaybeQueueFunctor);
mMaybeQueueFunctor.postProcess();
mMaybeQueueFunctor.postProcessLocked();
}
/** Returns true if both the calling and source users for the job are started. */

231
services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java
View File

@@ -30,6 +30,7 @@ import static com.android.server.job.JobSchedulerService.RARE_INDEX;
import static com.android.server.job.JobSchedulerService.sElapsedRealtimeClock;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
@@ -54,6 +55,9 @@ import android.os.Looper;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.os.SystemClock;
import android.util.Log;
import android.util.SparseBooleanArray;
import android.util.SparseLongArray;
import com.android.server.AppStateTracker;
import com.android.server.AppStateTrackerImpl;
@@ -75,8 +79,11 @@ import org.mockito.quality.Strictness;
import java.time.Clock;
import java.time.Duration;
import java.time.ZoneOffset;
import java.util.Random;
public class JobSchedulerServiceTest {
private static final String TAG = JobSchedulerServiceTest.class.getSimpleName();
private JobSchedulerService mService;
private MockitoSession mMockingSession;
@@ -178,12 +185,21 @@ public class JobSchedulerServiceTest {
}
private static JobInfo.Builder createJobInfo() {
return new JobInfo.Builder(351, new ComponentName("foo", "bar"));
return createJobInfo(351);
}
private static JobInfo.Builder createJobInfo(int jobId) {
return new JobInfo.Builder(jobId, new ComponentName("foo", "bar"));
}
private JobStatus createJobStatus(String testTag, JobInfo.Builder jobInfoBuilder) {
return createJobStatus(testTag, jobInfoBuilder, 1234);
}
private JobStatus createJobStatus(String testTag, JobInfo.Builder jobInfoBuilder,
int callingUid) {
return JobStatus.createFromJobInfo(
jobInfoBuilder.build(), 1234, "com.android.test", 0, testTag);
jobInfoBuilder.build(), callingUid, "com.android.test", 0, testTag);
}
/**
@@ -716,7 +732,7 @@ public class JobSchedulerServiceTest {
assertEquals(i + 1, maybeQueueFunctor.runnableJobs.size());
assertEquals(sElapsedRealtimeClock.millis(), job.getFirstForceBatchedTimeElapsed());
}
maybeQueueFunctor.postProcess();
maybeQueueFunctor.postProcessLocked();
assertEquals(0, mService.mPendingJobs.size());
// Enough RARE jobs to run.
@@ -728,7 +744,7 @@ public class JobSchedulerServiceTest {
assertEquals(i + 1, maybeQueueFunctor.runnableJobs.size());
assertEquals(sElapsedRealtimeClock.millis(), job.getFirstForceBatchedTimeElapsed());
}
maybeQueueFunctor.postProcess();
maybeQueueFunctor.postProcessLocked();
assertEquals(5, mService.mPendingJobs.size());
// Not enough RARE jobs to run, but a non-batched job saves the day.
@@ -745,7 +761,7 @@ public class JobSchedulerServiceTest {
assertEquals(sElapsedRealtimeClock.millis(), job.getFirstForceBatchedTimeElapsed());
}
maybeQueueFunctor.accept(activeJob);
maybeQueueFunctor.postProcess();
maybeQueueFunctor.postProcessLocked();
assertEquals(3, mService.mPendingJobs.size());
// Not enough RARE jobs to run, but an old RARE job saves the day.
@@ -764,7 +780,7 @@ public class JobSchedulerServiceTest {
}
maybeQueueFunctor.accept(oldRareJob);
assertEquals(oldBatchTime, oldRareJob.getFirstForceBatchedTimeElapsed());
maybeQueueFunctor.postProcess();
maybeQueueFunctor.postProcessLocked();
assertEquals(3, mService.mPendingJobs.size());
}
@@ -853,4 +869,207 @@ public class JobSchedulerServiceTest {
0, ""));
}
}
@Test
public void testPendingJobSorting() {
// First letter in job variable name indicate regular (r) or expedited (e).
// Capital letters in job variable name indicate the app/UID.
// Numbers in job variable name indicate the enqueue time.
// Expected sort order:
// eA7 > rA1 > eB6 > rB2 > eC3 > rD4 > eE5 > eF9 > rF8 > eC11 > rC10 > rG12 > rG13 > eE14
// Intentions:
// * A jobs let us test skipping both regular and expedited jobs of other apps
// * B jobs let us test skipping only regular job of another app without going too far
// * C jobs test that regular jobs don't skip over other app's jobs and that EJs only
// skip up to level of the earliest regular job
// * 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
JobStatus rA1 = createJobStatus("testPendingJobSorting", createJobInfo(1), 1);
JobStatus rB2 = createJobStatus("testPendingJobSorting", createJobInfo(2), 2);
JobStatus eC3 = createJobStatus("testPendingJobSorting",
createJobInfo(3).setExpedited(true), 3);
JobStatus rD4 = createJobStatus("testPendingJobSorting", createJobInfo(4), 4);
JobStatus eE5 = createJobStatus("testPendingJobSorting",
createJobInfo(5).setExpedited(true), 5);
JobStatus eB6 = createJobStatus("testPendingJobSorting",
createJobInfo(6).setExpedited(true), 2);
JobStatus eA7 = createJobStatus("testPendingJobSorting",
createJobInfo(7).setExpedited(true), 1);
JobStatus rF8 = createJobStatus("testPendingJobSorting", createJobInfo(8), 6);
JobStatus eF9 = createJobStatus("testPendingJobSorting",
createJobInfo(9).setExpedited(true), 6);
JobStatus rC10 = createJobStatus("testPendingJobSorting", createJobInfo(10), 3);
JobStatus eC11 = createJobStatus("testPendingJobSorting",
createJobInfo(11).setExpedited(true), 3);
JobStatus rG12 = createJobStatus("testPendingJobSorting", createJobInfo(12), 7);
JobStatus rG13 = createJobStatus("testPendingJobSorting", createJobInfo(13), 7);
JobStatus eE14 = createJobStatus("testPendingJobSorting",
createJobInfo(14).setExpedited(true), 5);
rA1.enqueueTime = 1;
rB2.enqueueTime = 2;
eC3.enqueueTime = 3;
rD4.enqueueTime = 4;
eE5.enqueueTime = 5;
eB6.enqueueTime = 6;
eA7.enqueueTime = 7;
rF8.enqueueTime = 8;
eF9.enqueueTime = 9;
rC10.enqueueTime = 10;
eC11.enqueueTime = 11;
rG12.enqueueTime = 12;
rG13.enqueueTime = 13;
eE14.enqueueTime = 14;
mService.mPendingJobs.clear();
// Add in random order so sorting is apparent.
mService.mPendingJobs.add(eC3);
mService.mPendingJobs.add(eE5);
mService.mPendingJobs.add(rA1);
mService.mPendingJobs.add(rG13);
mService.mPendingJobs.add(rD4);
mService.mPendingJobs.add(eA7);
mService.mPendingJobs.add(rG12);
mService.mPendingJobs.add(rF8);
mService.mPendingJobs.add(eB6);
mService.mPendingJobs.add(eE14);
mService.mPendingJobs.add(eF9);
mService.mPendingJobs.add(rB2);
mService.mPendingJobs.add(rC10);
mService.mPendingJobs.add(eC11);
mService.mPendingJobComparator.refreshLocked();
mService.mPendingJobs.sort(mService.mPendingJobComparator);
final JobStatus[] expectedOrder = new JobStatus[]{
eA7, rA1, eB6, rB2, eC3, rD4, eE5, eF9, 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());
}
}
private void checkPendingJobInvariants() {
long regJobEnqueueTime = 0;
final SparseBooleanArray regJobSeen = new SparseBooleanArray();
final SparseLongArray ejEnqueueTimes = new SparseLongArray();
for (int i = 0; i < mService.mPendingJobs.size(); ++i) {
final JobStatus job = mService.mPendingJobs.get(i);
final int uid = job.getSourceUid();
if (!job.isRequestedExpeditedJob()) {
// Invariant #1: Regular jobs are sorted by enqueue time.
assertTrue("Regular job with earlier enqueue time sorted after a later time: "
+ regJobEnqueueTime + " vs " + job.enqueueTime,
regJobEnqueueTime <= job.enqueueTime);
regJobEnqueueTime = job.enqueueTime;
regJobSeen.put(uid, true);
} else {
// Invariant #2: EJs should be before regular jobs for an individual app
if (regJobSeen.get(uid)) {
fail("UID " + uid + " had an EJ ordered after a regular job");
}
final long ejEnqueueTime = ejEnqueueTimes.get(uid, 0);
// Invariant #3: EJs for an individual app should be sorted by enqueue time.
assertTrue("EJ with earlier enqueue time sorted after a later time: "
+ ejEnqueueTime + " vs " + job.enqueueTime,
ejEnqueueTime <= job.enqueueTime);
ejEnqueueTimes.put(uid, job.enqueueTime);
}
}
}
private static String sortedJobToString(JobStatus job) {
return "testJob " + job.getSourceUid() + "/" + job.getJobId() + "/"
+ job.isRequestedExpeditedJob() + "@" + job.enqueueTime;
}
@Test
public void testPendingJobSorting_Random() {
Random random = new Random(1); // Always use the same series of pseudo random values.
mService.mPendingJobs.clear();
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));
mService.mPendingJobs.add(job);
mService.mPendingJobComparator.refreshLocked();
try {
mService.mPendingJobs.sort(mService.mPendingJobComparator);
} catch (Exception e) {
for (JobStatus toDump : mService.mPendingJobs) {
Log.i(TAG, sortedJobToString(toDump));
}
throw e;
}
checkPendingJobInvariants();
}
}
private int sign(int i) {
if (i > 0) {
return 1;
}
if (i < 0) {
return -1;
}
return 0;
}
@Test
public void testPendingJobSortingTransitivity() {
Random random = new Random(1); // Always use the same series of pseudo random values.
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);
}
mService.mPendingJobComparator.refreshLocked();
for (int i = 0; i < mService.mPendingJobs.size(); ++i) {
final JobStatus job1 = mService.mPendingJobs.get(i);
for (int j = 0; j < mService.mPendingJobs.size(); ++j) {
final JobStatus job2 = mService.mPendingJobs.get(j);
final int sign12 = sign(mService.mPendingJobComparator.compare(job1, job2));
final int sign21 = sign(mService.mPendingJobComparator.compare(job2, job1));
if (sign12 != -sign21) {
final String job1String = sortedJobToString(job1);
final String job2String = sortedJobToString(job2);
fail("compare(" + job1String + ", " + job2String + ") != "
+ "-compare(" + job2String + ", " + job1String + ")");
}
for (int k = 0; k < mService.mPendingJobs.size(); ++k) {
final JobStatus job3 = mService.mPendingJobs.get(k);
final int sign23 = sign(mService.mPendingJobComparator.compare(job2, job3));
final int sign13 = sign(mService.mPendingJobComparator.compare(job1, job3));
// Confirm 1 < 2 < 3 or 1 > 2 > 3 or 1 == 2 == 3
if ((sign12 == sign23 && sign12 != sign13)
// Confirm that if 1 == 2, then (1 < 3 AND 2 < 3) OR (1 > 3 && 2 > 3)
|| (sign12 == 0 && sign13 != sign23)) {
final String job1String = sortedJobToString(job1);
final String job2String = sortedJobToString(job2);
final String job3String = sortedJobToString(job3);
fail("Transitivity fail"
+ ": compare(" + job1String + ", " + job2String + ")=" + sign12
+ ", compare(" + job2String + ", " + job3String + ")=" + sign23
+ ", compare(" + job1String + ", " + job3String + ")=" + sign13);
}
}
}
}
}
}