diff --git a/apex/jobscheduler/service/java/com/android/server/job/PendingJobQueue.java b/apex/jobscheduler/service/java/com/android/server/job/PendingJobQueue.java index 993e178de0eef..e3426743c0a4d 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/PendingJobQueue.java +++ b/apex/jobscheduler/service/java/com/android/server/job/PendingJobQueue.java @@ -54,6 +54,13 @@ class PendingJobQueue { } else if (t2 == AppJobQueue.NO_NEXT_TIMESTAMP) { return -1; } + final int o1 = ajq1.peekNextOverrideState(); + final int o2 = ajq2.peekNextOverrideState(); + if (o1 != o2) { + // Higher override state (OVERRIDE_FULL) should be before lower state + // (OVERRIDE_SOFT) + return Integer.compare(o2, o1); + } return Long.compare(t1, t2); }); @@ -185,6 +192,7 @@ class PendingJobQueue { private static final class AppJobQueue { static final long NO_NEXT_TIMESTAMP = -1L; + static final int NO_NEXT_OVERRIDE_STATE = -1; private static class AdjustedJobStatus { public long adjustedEnqueueTime; @@ -207,7 +215,7 @@ class PendingJobQueue { if (job1.overrideState != job2.overrideState) { // Higher override state (OVERRIDE_FULL) should be before lower state // (OVERRIDE_SOFT) - return job2.overrideState - job1.overrideState; + return Integer.compare(job2.overrideState, job1.overrideState); } final boolean job1EJ = job1.isRequestedExpeditedJob(); @@ -223,18 +231,15 @@ class PendingJobQueue { if (job1Priority != job2Priority) { // Use the priority set by an app for intra-app job ordering. Higher // priority should be before lower priority. - return job2Priority - job1Priority; + return Integer.compare(job2Priority, job1Priority); } if (job1.lastEvaluatedBias != job2.lastEvaluatedBias) { // Higher bias should go first. - return job2.lastEvaluatedBias - job1.lastEvaluatedBias; + return Integer.compare(job2.lastEvaluatedBias, job1.lastEvaluatedBias); } - if (job1.enqueueTime < job2.enqueueTime) { - return -1; - } - return job1.enqueueTime > job2.enqueueTime ? 1 : 0; + return Long.compare(job1.enqueueTime, job2.enqueueTime); }; private static final Pools.Pool mAdjustedJobStatusPool = @@ -344,9 +349,14 @@ class PendingJobQueue { if (mCurIndex >= mJobs.size()) { return null; } - JobStatus next = mJobs.get(mCurIndex).job; - mCurIndex++; - return next; + return mJobs.get(mCurIndex++).job; + } + + int peekNextOverrideState() { + if (mCurIndex >= mJobs.size()) { + return NO_NEXT_OVERRIDE_STATE; + } + return mJobs.get(mCurIndex).job.overrideState; } long peekNextTimestamp() { diff --git a/services/tests/servicestests/src/com/android/server/job/PendingJobQueueTest.java b/services/tests/servicestests/src/com/android/server/job/PendingJobQueueTest.java index 150e3c60b91ee..bce6b1b2487e6 100644 --- a/services/tests/servicestests/src/com/android/server/job/PendingJobQueueTest.java +++ b/services/tests/servicestests/src/com/android/server/job/PendingJobQueueTest.java @@ -28,7 +28,6 @@ import android.platform.test.annotations.LargeTest; import android.util.Log; import android.util.SparseArray; import android.util.SparseBooleanArray; -import android.util.SparseIntArray; import android.util.SparseLongArray; import com.android.server.job.controllers.JobStatus; @@ -371,38 +370,34 @@ public class PendingJobQueueTest { private void checkPendingJobInvariants(PendingJobQueue jobQueue) { final SparseBooleanArray regJobSeen = new SparseBooleanArray(); - final SparseIntArray lastOverrideStateSeen = new SparseIntArray(); // Latest priority enqueue times seen for each priority for each app. final SparseArray latestPriorityRegEnqueueTimesPerUid = new SparseArray<>(); final SparseArray latestPriorityEjEnqueueTimesPerUid = new SparseArray<>(); final int noEntry = -1; + int prevOverrideState = noEntry; JobStatus job; jobQueue.resetIterator(); while ((job = jobQueue.next()) != null) { final int uid = job.getSourceUid(); - // Invariant #1: All jobs (for a UID) are sorted by override state + // Invariant #1: All jobs are sorted by override state // Invariant #2: All jobs (for a UID) are sorted by priority order // Invariant #3: Jobs (for a UID) with the same priority are sorted by enqueue time. // Invariant #4: EJs (for a UID) should be before regular jobs - final int prevOverrideState = lastOverrideStateSeen.get(uid, noEntry); - lastOverrideStateSeen.put(uid, job.overrideState); - if (prevOverrideState == noEntry) { - // First job for UID - continue; - } - // Invariant 1 if (prevOverrideState != job.overrideState) { - assertTrue(prevOverrideState > job.overrideState); - // Override state can make ordering weird. Clear the other cached states for this - // UID to avoid confusion in the other checks. - latestPriorityEjEnqueueTimesPerUid.remove(uid); - latestPriorityRegEnqueueTimesPerUid.remove(uid); - regJobSeen.delete(uid); + if (prevOverrideState != noEntry) { + assertTrue(prevOverrideState > job.overrideState); + } + // Override state can make ordering weird. Clear the other cached states + // to avoid confusion in the other checks. + latestPriorityEjEnqueueTimesPerUid.clear(); + latestPriorityRegEnqueueTimesPerUid.clear(); + regJobSeen.clear(); + prevOverrideState = job.overrideState; } final int priority = job.getEffectivePriority(); @@ -423,8 +418,9 @@ public class PendingJobQueueTest { final long lastSeenPriorityEnqueueTime = latestPriorityEnqueueTimes.get(priority, noEntry); if (lastSeenPriorityEnqueueTime != noEntry) { - assertTrue("Jobs with same priority not sorted by enqueue time: " - + lastSeenPriorityEnqueueTime + " vs " + job.enqueueTime, + assertTrue("Jobs with same priority for uid " + uid + + " not sorted by enqueue time: " + + lastSeenPriorityEnqueueTime + " before " + job.enqueueTime, lastSeenPriorityEnqueueTime <= job.enqueueTime); } } else {