From 16c0ede0b116257bf74e2e7475af29f39ddb17df Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 17 Mar 2022 20:28:02 +0000 Subject: [PATCH] Use new sorting mechanism. Switch the pending job sorting mechanism the new topological-sort-like mechanism. The new system is generally more efficient and easier to maintain than the previous implementation and allows for further improvements such as batching in the queue (to reduce process restarts). Runtime changes (A=# of apps, J=average # jobs per app): Previous implementation New implementation Sorting: A*J*log(A*J) A*J*log(J) Insertion: log(A*J) log(A*J)+J Remove(Object): A*J log(A*J) Iteration: A*J A*J (amortized) Contains: A*J log(A*J) Bug: 141645789 Bug: 204924801 Bug: 223437753 Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job Test: atest CtsJobSchedulerTestCases Change-Id: I61cf8488d574e3c7be624b753a02e8c1f151f4fd --- .../server/job/JobConcurrencyManager.java | 59 +-- .../server/job/JobSchedulerService.java | 257 ++---------- .../server/job/JobSchedulerServiceTest.java | 382 +----------------- 3 files changed, 75 insertions(+), 623 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java index 23056b5670b9..405b12d6d13a 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java @@ -68,7 +68,6 @@ import java.io.PrintWriter; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import java.util.function.Consumer; import java.util.function.Predicate; @@ -578,7 +577,7 @@ class JobConcurrencyManager { Slog.d(TAG, printPendingQueueLocked()); } - final List pendingJobs = mService.mPendingJobs; + final PendingJobQueue pendingJobQueue = mService.mPendingJobQueue; final List activeServices = mActiveServices; // To avoid GC churn, we recycle the arrays. @@ -597,7 +596,7 @@ class JobConcurrencyManager { // Update the priorities of jobs that aren't running, and also count the pending work types. // Do this before the following loop to hopefully reduce the cost of // shouldStopRunningJobLocked(). - updateNonRunningPrioritiesLocked(pendingJobs, true); + updateNonRunningPrioritiesLocked(pendingJobQueue, true); for (int i = 0; i < MAX_JOB_CONTEXTS_COUNT; i++) { final JobServiceContext js = activeServices.get(i); @@ -620,9 +619,9 @@ class JobConcurrencyManager { mWorkCountTracker.onCountDone(); - for (int i = 0; i < pendingJobs.size(); i++) { - final JobStatus nextPending = pendingJobs.get(i); - + JobStatus nextPending; + pendingJobQueue.resetIterator(); + while ((nextPending = pendingJobQueue.next()) != null) { if (mRunningJobs.contains(nextPending)) { continue; } @@ -841,10 +840,11 @@ class JobConcurrencyManager { } @GuardedBy("mLock") - private void updateNonRunningPrioritiesLocked(@NonNull final List pendingJobs, + private void updateNonRunningPrioritiesLocked(@NonNull final PendingJobQueue jobQueue, boolean updateCounter) { - for (int i = 0; i < pendingJobs.size(); i++) { - final JobStatus pending = pendingJobs.get(i); + JobStatus pending; + jobQueue.resetIterator(); + while ((pending = jobQueue.next()) != null) { // If job is already running, go to next job. if (mRunningJobs.contains(pending)) { @@ -882,7 +882,8 @@ class JobConcurrencyManager { } // Use < instead of <= as that gives us a little wiggle room in case a new job comes // along very shortly. - if (mService.mPendingJobs.size() + mRunningJobs.size() < mWorkTypeConfig.getMaxTotal()) { + if (mService.mPendingJobQueue.size() + mRunningJobs.size() + < mWorkTypeConfig.getMaxTotal()) { // Don't artificially limit a single package if we don't even have enough jobs to use // the maximum number of slots. We'll preempt the job later if we need the slot. return false; @@ -936,8 +937,7 @@ class JobConcurrencyManager { jobStatus.getSourceUserId(), jobStatus.getSourcePackageName(), packageStats); } - final List pendingJobs = mService.mPendingJobs; - if (pendingJobs.remove(jobStatus)) { + if (mService.mPendingJobQueue.remove(jobStatus)) { mService.mJobPackageTracker.noteNonpending(jobStatus); } } finally { @@ -962,11 +962,11 @@ class JobConcurrencyManager { } } - final List pendingJobs = mService.mPendingJobs; + final PendingJobQueue pendingJobQueue = mService.mPendingJobQueue; if (worker.getPreferredUid() != JobServiceContext.NO_PREFERRED_UID) { updateCounterConfigLocked(); // Preemption case needs special care. - updateNonRunningPrioritiesLocked(pendingJobs, false); + updateNonRunningPrioritiesLocked(pendingJobQueue, false); JobStatus highestBiasJob = null; int highBiasWorkType = workType; @@ -974,9 +974,10 @@ class JobConcurrencyManager { JobStatus backupJob = null; int backupWorkType = WORK_TYPE_NONE; int backupAllWorkTypes = WORK_TYPE_NONE; - for (int i = 0; i < pendingJobs.size(); i++) { - final JobStatus nextPending = pendingJobs.get(i); + JobStatus nextPending; + pendingJobQueue.resetIterator(); + while ((nextPending = pendingJobQueue.next()) != null) { if (mRunningJobs.contains(nextPending)) { continue; } @@ -1041,16 +1042,18 @@ class JobConcurrencyManager { startJobLocked(worker, backupJob, backupWorkType); } } - } else if (pendingJobs.size() > 0) { + } else if (pendingJobQueue.size() > 0) { updateCounterConfigLocked(); - updateNonRunningPrioritiesLocked(pendingJobs, false); + updateNonRunningPrioritiesLocked(pendingJobQueue, false); // This slot is now free and we have pending jobs. Start the highest bias job we find. JobStatus highestBiasJob = null; int highBiasWorkType = workType; int highBiasAllWorkTypes = workType; - for (int i = 0; i < pendingJobs.size(); i++) { - final JobStatus nextPending = pendingJobs.get(i); + + JobStatus nextPending; + pendingJobQueue.resetIterator(); + while ((nextPending = pendingJobQueue.next()) != null) { if (mRunningJobs.contains(nextPending)) { continue; @@ -1127,8 +1130,8 @@ class JobConcurrencyManager { return "too many jobs running"; } - final List pendingJobs = mService.mPendingJobs; - final int numPending = pendingJobs.size(); + final PendingJobQueue pendingJobQueue = mService.mPendingJobQueue; + final int numPending = pendingJobQueue.size(); if (numPending == 0) { // All quiet. We can let this job run to completion. return null; @@ -1163,8 +1166,9 @@ class JobConcurrencyManager { // Harder check. We need to see if a different work type can replace this job. int remainingWorkTypes = ALL_WORK_TYPES; - for (int i = 0; i < numPending; ++i) { - final JobStatus pending = pendingJobs.get(i); + JobStatus pending; + pendingJobQueue.resetIterator(); + while ((pending = pendingJobQueue.next()) != null) { final int workTypes = getJobWorkTypes(pending); if ((workTypes & remainingWorkTypes) > 0 && mWorkCountTracker.canJobStart(workTypes, workType) != WORK_TYPE_NONE) { @@ -1201,9 +1205,10 @@ class JobConcurrencyManager { @GuardedBy("mLock") private String printPendingQueueLocked() { StringBuilder s = new StringBuilder("Pending queue: "); - Iterator it = mService.mPendingJobs.iterator(); - while (it.hasNext()) { - JobStatus js = it.next(); + PendingJobQueue pendingJobQueue = mService.mPendingJobQueue; + JobStatus js; + pendingJobQueue.resetIterator(); + while ((js = pendingJobQueue.next()) != null) { s.append("(") .append(js.getJob().getId()) .append(", ") diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index 3d74bc98ad32..2028be738238 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -21,7 +21,6 @@ import static android.content.pm.PackageManager.COMPONENT_ENABLED_STATE_DISABLED import static android.text.format.DateUtils.HOUR_IN_MILLIS; import static android.text.format.DateUtils.MINUTE_IN_MILLIS; -import android.annotation.ElapsedRealtimeLong; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; @@ -81,7 +80,6 @@ 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; @@ -288,7 +286,7 @@ public class JobSchedulerService extends com.android.server.SystemService * Queue of pending jobs. The JobServiceContext class will receive jobs from this list * when ready to execute them. */ - final ArrayList mPendingJobs = new ArrayList<>(); + final PendingJobQueue mPendingJobQueue = new PendingJobQueue(); int[] mStartedUsers = EmptyArray.INT; @@ -828,189 +826,6 @@ public class JobSchedulerService extends com.android.server.SystemService final Constants mConstants; final ConstantsObserver mConstantsObserver; - @VisibleForTesting - class PendingJobComparator implements Comparator { - private static final int EJ_PRIORITY_MODIFIER = 10; - - /** Cache of the earliest non-PRIORITY_MAX enqueue time found per UID. */ - private final SparseLongArray mEarliestNonMaxEnqueueTimeCache = new SparseLongArray(); - /** - * Cache of the last enqueue time of each priority for each UID. The SparseArray is keyed - * by UID and the SparseLongArray is keyed by the priority. - */ - private final SparseArray mLastPriorityEnqueueTimeCache = - new SparseArray<>(); - /** - * The earliest enqueue time each UID's priority's jobs should use. The SparseArray is keyed - * by UID and the SparseLongArray is keyed by the value returned from - * {@link #getPriorityIndex(int, boolean)}. - */ - private final SparseArray mEarliestAllowedEnqueueTimes = - new SparseArray<>(); - - private int getPriorityIndex(int priority, boolean isEJ) { - // We need to separate HIGH priority EJs from HIGH priority regular jobs. - if (isEJ) { - return priority * EJ_PRIORITY_MODIFIER; - } - return priority; - } - - /** - * Refresh sorting determinants based on the current state of {@link #mPendingJobs}. - */ - @GuardedBy("mLock") - @VisibleForTesting - void refreshLocked() { - mEarliestNonMaxEnqueueTimeCache.clear(); - for (int i = 0; i < mPendingJobs.size(); ++i) { - final JobStatus job = mPendingJobs.get(i); - final int uid = job.getSourceUid(); - if (job.getEffectivePriority() < JobInfo.PRIORITY_MAX) { - final long earliestEnqueueTime = - mEarliestNonMaxEnqueueTimeCache.get(uid, Long.MAX_VALUE); - mEarliestNonMaxEnqueueTimeCache.put(uid, - Math.min(earliestEnqueueTime, job.enqueueTime)); - } - - final int pIdx = - getPriorityIndex(job.getEffectivePriority(), job.isRequestedExpeditedJob()); - SparseLongArray lastPriorityEnqueueTime = mLastPriorityEnqueueTimeCache.get(uid); - if (lastPriorityEnqueueTime == null) { - lastPriorityEnqueueTime = new SparseLongArray(); - mLastPriorityEnqueueTimeCache.put(uid, lastPriorityEnqueueTime); - } - lastPriorityEnqueueTime.put(pIdx, - Math.max(job.enqueueTime, lastPriorityEnqueueTime.get(pIdx, 0))); - } - - // Move lower priority jobs behind higher priority jobs (instead of moving higher - // priority jobs ahead of lower priority jobs), except for EJs. - for (int i = 0; i < mLastPriorityEnqueueTimeCache.size(); ++i) { - final int uid = mLastPriorityEnqueueTimeCache.keyAt(i); - SparseLongArray lastEnqueueTimes = mLastPriorityEnqueueTimeCache.valueAt(i); - SparseLongArray earliestAllowedEnqueueTimes = new SparseLongArray(); - mEarliestAllowedEnqueueTimes.put(uid, earliestAllowedEnqueueTimes); - long earliestAllowedEnqueueTime = mEarliestNonMaxEnqueueTimeCache.get(uid, - lastEnqueueTimes.get(getPriorityIndex(JobInfo.PRIORITY_MAX, true), -1)); - earliestAllowedEnqueueTimes.put(getPriorityIndex(JobInfo.PRIORITY_MAX, true), - earliestAllowedEnqueueTime); - earliestAllowedEnqueueTime = 1 - + Math.max(earliestAllowedEnqueueTime, - lastEnqueueTimes.get(getPriorityIndex(JobInfo.PRIORITY_HIGH, true), -1)); - earliestAllowedEnqueueTimes.put(getPriorityIndex(JobInfo.PRIORITY_HIGH, true), - earliestAllowedEnqueueTime); - earliestAllowedEnqueueTime++; - for (int p = JobInfo.PRIORITY_HIGH; p >= JobInfo.PRIORITY_MIN; --p) { - final int pIdx = getPriorityIndex(p, false); - earliestAllowedEnqueueTimes.put(pIdx, earliestAllowedEnqueueTime); - final long lastEnqueueTime = lastEnqueueTimes.get(pIdx, -1); - if (lastEnqueueTime != -1) { - // Add additional millisecond for the next priority to ensure sorting is - // stable/accurate when comparing to other apps. - earliestAllowedEnqueueTime = 1 - + Math.max(earliestAllowedEnqueueTime, lastEnqueueTime); - } - } - } - - // Clear intermediate state that we don't need to reduce steady state memory usage. - mLastPriorityEnqueueTimeCache.clear(); - } - - @ElapsedRealtimeLong - private long getEffectiveEnqueueTime(@NonNull JobStatus job) { - // Move lower priority jobs behind higher priority jobs (instead of moving higher - // priority jobs ahead of lower priority jobs), except for MAX EJs. - final int uid = job.getSourceUid(); - if (job.isRequestedExpeditedJob() - && job.getEffectivePriority() == JobInfo.PRIORITY_MAX) { - return Math.min(job.enqueueTime, - mEarliestNonMaxEnqueueTimeCache.get(uid, Long.MAX_VALUE)); - } - final int priorityIdx = - getPriorityIndex(job.getEffectivePriority(), job.isRequestedExpeditedJob()); - final SparseLongArray earliestAllowedEnqueueTimes = - mEarliestAllowedEnqueueTimes.get(uid); - if (earliestAllowedEnqueueTimes == null) { - // We're probably trying to insert directly without refreshing the internal arrays. - // Since we haven't seen this UID before, we can just use the job's enqueue time. - return job.enqueueTime; - } - return Math.max(job.enqueueTime, earliestAllowedEnqueueTimes.get(priorityIdx)); - } - - @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; - } - if (o1.getEffectivePriority() != o2.getEffectivePriority()) { - // Use the priority set by an app for intra-app job ordering. Higher - // priority should be before lower priority. - return o2.getEffectivePriority() - o1.getEffectivePriority(); - } - } else { - // TODO: see if we can simplify this using explicit topological sorting - // Since we order jobs within a UID by the job's priority, in order to satisfy the - // transitivity constraint of the comparator, we must ensure consistent/appropriate - // ordering between apps as well. That is, if a job is ordered before or behind - // another job because of its priority, that ordering must translate to the - // relative ordering against other jobs. - // The effective ordering implementation here is to use HIGH priority EJs as a - // pivot point. MAX priority EJs are moved *ahead* of HIGH priority EJs. All - // regular jobs are moved *behind* HIGH priority EJs. The intention for moving jobs - // "behind" the EJs instead of moving all high priority jobs before lower priority - // jobs is to reduce any potential abuse (or just unfortunate execution) cases where - // there are early low priority jobs that don't get to run because so many of the - // app's high priority jobs are pushed before low priority job. This may still - // happen because of the job ordering mechanism, but moving jobs back prevents - // one app's jobs from always being at the front (due to the early scheduled low - // priority job and our base case of sorting by enqueue time). - - final long o1EffectiveEnqueueTime = getEffectiveEnqueueTime(o1); - final long o2EffectiveEnqueueTime = getEffectiveEnqueueTime(o2); - - if (o1EffectiveEnqueueTime < o2EffectiveEnqueueTime) { - return -1; - } else if (o1EffectiveEnqueueTime > o2EffectiveEnqueueTime) { - return 1; - } - } - - if (o1.enqueueTime < o2.enqueueTime) { - return -1; - } - return o1.enqueueTime > o2.enqueueTime ? 1 : 0; - } - } - - @VisibleForTesting - final PendingJobComparator mPendingJobComparator = new PendingJobComparator(); - - static void addOrderedItem(ArrayList array, T newItem, Comparator comparator) { - int where = Collections.binarySearch(array, newItem, comparator); - if (where < 0) { - where = ~where; - } - array.add(where, newItem); - } - /** * Cleans up outstanding jobs when a package is removed. Even if it's being replaced later we * still clean up. On reinstall the package will have a new uid. @@ -1434,7 +1249,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, mPendingJobComparator); + mPendingJobQueue.add(jobStatus); maybeRunPendingJobsLocked(); } else { evaluateControllerStatesLocked(jobStatus); @@ -1563,7 +1378,7 @@ public class JobSchedulerService extends com.android.server.SystemService cancelled.unprepareLocked(); stopTrackingJobLocked(cancelled, incomingJob, true /* writeBack */); // Remove from pending queue. - if (mPendingJobs.remove(cancelled)) { + if (mPendingJobQueue.remove(cancelled)) { mJobPackageTracker.noteNonpending(cancelled); } // Cancel if running. @@ -1658,8 +1473,8 @@ public class JobSchedulerService extends com.android.server.SystemService void reportActiveLocked() { // active is true if pending queue contains jobs OR some job is running. - boolean active = mPendingJobs.size() > 0; - if (mPendingJobs.size() <= 0) { + boolean active = mPendingJobQueue.size() > 0; + if (!active) { final ArraySet runningJobs = mConcurrencyManager.getRunningJobsLocked(); for (int i = runningJobs.size() - 1; i >= 0; --i) { final JobStatus job = runningJobs.valueAt(i); @@ -1952,10 +1767,13 @@ public class JobSchedulerService extends com.android.server.SystemService mJobPackageTracker.noteNonpending(job); } - void noteJobsNonpending(List jobs) { - for (int i = jobs.size() - 1; i >= 0; i--) { - noteJobNonPending(jobs.get(i)); + private void clearPendingJobQueue() { + JobStatus job; + mPendingJobQueue.resetIterator(); + while ((job = mPendingJobQueue.next()) != null) { + noteJobNonPending(job); } + mPendingJobQueue.clear(); } /** @@ -2236,7 +2054,7 @@ public class JobSchedulerService extends com.android.server.SystemService if (js != null) { if (isReadyToBeExecutedLocked(js)) { mJobPackageTracker.notePending(js); - addOrderedItem(mPendingJobs, js, mPendingJobComparator); + mPendingJobQueue.add(js); } mChangedJobList.remove(js); } else { @@ -2382,14 +2200,13 @@ public class JobSchedulerService extends com.android.server.SystemService if (DEBUG) { Slog.d(TAG, "queuing all ready jobs for execution:"); } - noteJobsNonpending(mPendingJobs); - mPendingJobs.clear(); + clearPendingJobQueue(); stopNonReadyActiveJobsLocked(); mJobs.forEachJob(mReadyQueueFunctor); mReadyQueueFunctor.postProcessLocked(); if (DEBUG) { - final int queuedJobs = mPendingJobs.size(); + final int queuedJobs = mPendingJobQueue.size(); if (queuedJobs == 0) { Slog.d(TAG, "No jobs pending."); } else { @@ -2416,11 +2233,7 @@ public class JobSchedulerService extends com.android.server.SystemService @GuardedBy("mLock") private void postProcessLocked() { noteJobsPending(newReadyJobs); - mPendingJobs.addAll(newReadyJobs); - mPendingJobComparator.refreshLocked(); - if (mPendingJobs.size() > 1) { - mPendingJobs.sort(mPendingJobComparator); - } + mPendingJobQueue.addAll(newReadyJobs); newReadyJobs.clear(); } @@ -2453,7 +2266,7 @@ public class JobSchedulerService extends com.android.server.SystemService mHandler.obtainMessage(MSG_STOP_JOB, JobParameters.STOP_REASON_BACKGROUND_RESTRICTION, 0, job) .sendToTarget(); - } else if (mPendingJobs.remove(job)) { + } else if (mPendingJobQueue.remove(job)) { noteJobNonPending(job); } return; @@ -2530,7 +2343,7 @@ public class JobSchedulerService extends com.android.server.SystemService } mConcurrencyManager.stopJobOnServiceContextLocked(job, job.getStopReason(), internalStopReason, debugReason); - } else if (mPendingJobs.remove(job)) { + } else if (mPendingJobQueue.remove(job)) { noteJobNonPending(job); } evaluateControllerStatesLocked(job); @@ -2546,11 +2359,7 @@ public class JobSchedulerService extends com.android.server.SystemService Slog.d(TAG, "maybeQueueReadyJobsForExecutionLocked: Running jobs."); } noteJobsPending(runnableJobs); - mPendingJobs.addAll(runnableJobs); - mPendingJobComparator.refreshLocked(); - if (mPendingJobs.size() > 1) { - mPendingJobs.sort(mPendingJobComparator); - } + mPendingJobQueue.addAll(runnableJobs); } else { if (DEBUG) { Slog.d(TAG, "maybeQueueReadyJobsForExecutionLocked: Not running anything."); @@ -2574,14 +2383,13 @@ public class JobSchedulerService extends com.android.server.SystemService @GuardedBy("mLock") private void maybeQueueReadyJobsForExecutionLocked() { mHandler.removeMessages(MSG_CHECK_JOB); - // This method will evaluate all jobs, so we don't need to keep any messages for a suubset + // This method will evaluate all jobs, so we don't need to keep any messages for a subset // of jobs in the queue. mHandler.removeMessages(MSG_CHECK_CHANGED_JOB_LIST); mChangedJobList.clear(); if (DEBUG) Slog.d(TAG, "Maybe queuing ready jobs..."); - noteJobsNonpending(mPendingJobs); - mPendingJobs.clear(); + clearPendingJobQueue(); stopNonReadyActiveJobsLocked(); mJobs.forEachJob(mMaybeQueueFunctor); mMaybeQueueFunctor.postProcessLocked(); @@ -2682,7 +2490,7 @@ public class JobSchedulerService extends com.android.server.SystemService return false; } - final boolean jobPending = mPendingJobs.contains(job); + final boolean jobPending = mPendingJobQueue.contains(job); final boolean jobActive = rejectActive && mConcurrencyManager.isJobRunningLocked(job); if (DEBUG) { @@ -2802,7 +2610,7 @@ public class JobSchedulerService extends com.android.server.SystemService */ void maybeRunPendingJobsLocked() { if (DEBUG) { - Slog.d(TAG, "pending queue: " + mPendingJobs.size() + " jobs."); + Slog.d(TAG, "pending queue: " + mPendingJobQueue.size() + " jobs."); } mConcurrencyManager.assignJobsToContextsLocked(); reportActiveLocked(); @@ -3634,7 +3442,7 @@ public class JobSchedulerService extends com.android.server.SystemService } boolean printed = false; - if (mPendingJobs.contains(js)) { + if (mPendingJobQueue.contains(js)) { pw.print("pending"); printed = true; } @@ -3836,7 +3644,7 @@ public class JobSchedulerService extends com.android.server.SystemService pw.print(" !restricted="); pw.print(!isRestricted); pw.print(" !pending="); - pw.print(!mPendingJobs.contains(job)); + pw.print(!mPendingJobQueue.contains(job)); pw.print(" !active="); pw.print(!mConcurrencyManager.isJobRunningLocked(job)); pw.print(" !backingup="); @@ -3929,8 +3737,11 @@ public class JobSchedulerService extends com.android.server.SystemService boolean pendingPrinted = false; pw.println("Pending queue:"); pw.increaseIndent(); - for (int i = 0; i < mPendingJobs.size(); i++) { - JobStatus job = mPendingJobs.get(i); + JobStatus job; + int pendingIdx = 0; + mPendingJobQueue.resetIterator(); + while ((job = mPendingJobQueue.next()) != null) { + pendingIdx++; if (!predicate.test(job)) { continue; } @@ -3938,7 +3749,7 @@ public class JobSchedulerService extends com.android.server.SystemService pendingPrinted = true; } - pw.print("Pending #"); pw.print(i); pw.print(": "); + pw.print("Pending #"); pw.print(pendingIdx); pw.print(": "); pw.println(job.toShortString()); pw.increaseIndent(); @@ -3969,7 +3780,7 @@ public class JobSchedulerService extends com.android.server.SystemService // Print most recent first final int idx = (mLastCompletedJobIndex + NUM_COMPLETED_JOB_HISTORY - r) % NUM_COMPLETED_JOB_HISTORY; - final JobStatus job = mLastCompletedJobs[idx]; + job = mLastCompletedJobs[idx]; if (job != null) { if (!predicate.test(job)) { continue; @@ -4062,7 +3873,7 @@ public class JobSchedulerService extends com.android.server.SystemService JobSchedulerServiceDumpProto.RegisteredJob.IS_JOB_RESTRICTED, checkIfRestricted(job) != null); proto.write(JobSchedulerServiceDumpProto.RegisteredJob.IS_JOB_PENDING, - mPendingJobs.contains(job)); + mPendingJobQueue.contains(job)); proto.write(JobSchedulerServiceDumpProto.RegisteredJob.IS_JOB_CURRENTLY_ACTIVE, mConcurrencyManager.isJobRunningLocked(job)); proto.write(JobSchedulerServiceDumpProto.RegisteredJob.IS_UID_BACKING_UP, @@ -4109,7 +3920,9 @@ public class JobSchedulerService extends com.android.server.SystemService mJobPackageTracker.dumpHistory(proto, JobSchedulerServiceDumpProto.HISTORY, filterAppId); - for (JobStatus job : mPendingJobs) { + JobStatus job; + mPendingJobQueue.resetIterator(); + while ((job = mPendingJobQueue.next()) != null) { final long pjToken = proto.start(JobSchedulerServiceDumpProto.PENDING_JOBS); job.writeToShortProto(proto, PendingJob.INFO); diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java index f9bdad6c62ba..296e8a3bb563 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java @@ -30,7 +30,6 @@ 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.anyBoolean; @@ -56,11 +55,6 @@ import android.os.Looper; import android.os.RemoteException; import android.os.ServiceManager; import android.os.SystemClock; -import android.platform.test.annotations.LargeTest; -import android.util.Log; -import android.util.SparseArray; -import android.util.SparseBooleanArray; -import android.util.SparseLongArray; import com.android.server.AppStateTracker; import com.android.server.AppStateTrackerImpl; @@ -82,16 +76,10 @@ 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 static final int[] sRegJobPriorities = { - JobInfo.PRIORITY_HIGH, JobInfo.PRIORITY_DEFAULT, - JobInfo.PRIORITY_LOW, JobInfo.PRIORITY_MIN - }; - private JobSchedulerService mService; private MockitoSession mMockingSession; @@ -769,7 +757,7 @@ public class JobSchedulerServiceTest { job.setStandbyBucket(RARE_INDEX); // Not enough RARE jobs to run. - mService.mPendingJobs.clear(); + mService.mPendingJobQueue.clear(); maybeQueueFunctor.reset(); for (int i = 0; i < mService.mConstants.MIN_READY_NON_ACTIVE_JOBS_COUNT / 2; ++i) { maybeQueueFunctor.accept(job); @@ -778,10 +766,10 @@ public class JobSchedulerServiceTest { assertEquals(sElapsedRealtimeClock.millis(), job.getFirstForceBatchedTimeElapsed()); } maybeQueueFunctor.postProcessLocked(); - assertEquals(0, mService.mPendingJobs.size()); + assertEquals(0, mService.mPendingJobQueue.size()); // Enough RARE jobs to run. - mService.mPendingJobs.clear(); + mService.mPendingJobQueue.clear(); maybeQueueFunctor.reset(); for (int i = 0; i < mService.mConstants.MIN_READY_NON_ACTIVE_JOBS_COUNT; ++i) { maybeQueueFunctor.accept(job); @@ -790,10 +778,10 @@ public class JobSchedulerServiceTest { assertEquals(sElapsedRealtimeClock.millis(), job.getFirstForceBatchedTimeElapsed()); } maybeQueueFunctor.postProcessLocked(); - assertEquals(5, mService.mPendingJobs.size()); + assertEquals(5, mService.mPendingJobQueue.size()); // Not enough RARE jobs to run, but a non-batched job saves the day. - mService.mPendingJobs.clear(); + mService.mPendingJobQueue.clear(); maybeQueueFunctor.reset(); JobStatus activeJob = createJobStatus( "testRareJobBatching", @@ -807,10 +795,10 @@ public class JobSchedulerServiceTest { } maybeQueueFunctor.accept(activeJob); maybeQueueFunctor.postProcessLocked(); - assertEquals(3, mService.mPendingJobs.size()); + assertEquals(3, mService.mPendingJobQueue.size()); // Not enough RARE jobs to run, but an old RARE job saves the day. - mService.mPendingJobs.clear(); + mService.mPendingJobQueue.clear(); maybeQueueFunctor.reset(); JobStatus oldRareJob = createJobStatus("testRareJobBatching", createJobInfo()); oldRareJob.setStandbyBucket(RARE_INDEX); @@ -826,7 +814,7 @@ public class JobSchedulerServiceTest { maybeQueueFunctor.accept(oldRareJob); assertEquals(oldBatchTime, oldRareJob.getFirstForceBatchedTimeElapsed()); maybeQueueFunctor.postProcessLocked(); - assertEquals(3, mService.mPendingJobs.size()); + assertEquals(3, mService.mPendingJobQueue.size()); } /** Tests that jobs scheduled by the app itself are counted towards scheduling limits. */ @@ -914,358 +902,4 @@ 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 - // * 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", - 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 rH8 = createJobStatus("testPendingJobSorting", createJobInfo(8), 8); - 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 = 10; - rB2.enqueueTime = 20; - eC3.enqueueTime = 30; - rD4.enqueueTime = 40; - eE5.enqueueTime = 50; - eB6.enqueueTime = 60; - eA7.enqueueTime = 70; - rF8.enqueueTime = 80; - rH8.enqueueTime = 80; - eF9.enqueueTime = 90; - rC10.enqueueTime = 100; - eC11.enqueueTime = 110; - rG12.enqueueTime = 120; - rG13.enqueueTime = 130; - eE14.enqueueTime = 140; - - 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(rH8); - 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, 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()); - } - } - - private void checkPendingJobInvariants() { - final SparseBooleanArray regJobSeen = new SparseBooleanArray(); - // Latest priority enqueue times seen for each priority for each app. - final SparseArray latestPriorityRegEnqueueTimesPerUid = - new SparseArray<>(); - final SparseArray latestPriorityEjEnqueueTimesPerUid = new SparseArray<>(); - final long noEntry = -1; - - for (int i = 0; i < mService.mPendingJobs.size(); ++i) { - final JobStatus job = mService.mPendingJobs.get(i); - final int uid = job.getSourceUid(); - - // Invariant #1: All jobs (for a UID) are sorted by priority order - // Invariant #2: Jobs (for a UID) with the same priority are sorted by enqueue time. - // Invariant #3: EJs (for a UID) should be before regular jobs - - final int priority = job.getEffectivePriority(); - final SparseArray latestPriorityEnqueueTimesPerUid = - job.isRequestedExpeditedJob() - ? latestPriorityEjEnqueueTimesPerUid - : latestPriorityRegEnqueueTimesPerUid; - SparseLongArray latestPriorityEnqueueTimes = latestPriorityEnqueueTimesPerUid.get(uid); - if (latestPriorityEnqueueTimes != null) { - // Invariant 1 - for (int p = priority - 1; p >= JobInfo.PRIORITY_MIN; --p) { - // If we haven't seen the priority, there shouldn't be an entry in the array. - assertEquals("Jobs not properly sorted by priority for uid " + uid, - noEntry, latestPriorityEnqueueTimes.get(p, noEntry)); - } - - // Invariant 2 - final long lastSeenPriorityEnqueueTime = - latestPriorityEnqueueTimes.get(priority, noEntry); - if (lastSeenPriorityEnqueueTime != noEntry) { - assertTrue("Jobs with same priority not sorted by enqueue time: " - + lastSeenPriorityEnqueueTime + " vs " + job.enqueueTime, - lastSeenPriorityEnqueueTime <= job.enqueueTime); - } - } else { - latestPriorityEnqueueTimes = new SparseLongArray(); - latestPriorityEnqueueTimesPerUid.put(uid, latestPriorityEnqueueTimes); - } - latestPriorityEnqueueTimes.put(priority, job.enqueueTime); - - // Invariant 3 - if (!job.isRequestedExpeditedJob()) { - regJobSeen.put(uid, true); - } else if (regJobSeen.get(uid)) { - fail("UID " + uid + " had an EJ ordered after a regular job"); - } - } - } - - private static String sortedJobToString(JobStatus job) { - return "testJob " + job.getSourceUid() + "/" + job.getJobId() - + "/p" + job.getEffectivePriority() - + "/" + 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 < 5000; ++i) { - JobStatus job = createJobStatus("testPendingJobSorting_Random", - createJobInfo(i).setExpedited(random.nextBoolean()), random.nextInt(250)); - job.enqueueTime = 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() { - // 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(); - - 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 - @LargeTest - 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(); - } - } - - @Test - public void testPendingJobSorting_Random_WithPriority() { - Random random = new Random(1); // Always use the same series of pseudo random values. - - mService.mPendingJobs.clear(); - - for (int i = 0; i < 5000; ++i) { - final boolean isEj = random.nextBoolean(); - final int priority; - if (isEj) { - priority = random.nextBoolean() ? JobInfo.PRIORITY_MAX : JobInfo.PRIORITY_HIGH; - } else { - priority = sRegJobPriorities[random.nextInt(sRegJobPriorities.length)]; - } - JobStatus job = createJobStatus("testPendingJobSorting_Random_WithPriority", - createJobInfo(i).setExpedited(isEj).setPriority(priority), - random.nextInt(250)); - job.enqueueTime = 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(); - } - - @Test - public void testPendingJobSortingTransitivity_WithPriority() { - // 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(); - - for (int i = 0; i < 300; ++i) { - final boolean isEj = random.nextBoolean(); - final int priority; - if (isEj) { - priority = random.nextBoolean() ? JobInfo.PRIORITY_MAX : JobInfo.PRIORITY_HIGH; - } else { - priority = sRegJobPriorities[random.nextInt(sRegJobPriorities.length)]; - } - JobStatus job = createJobStatus("testPendingJobSortingTransitivity_WithPriority", - createJobInfo(i).setExpedited(isEj).setPriority(priority), - random.nextInt(50)); - job.enqueueTime = random.nextInt(1_000_000); - job.overrideState = random.nextInt(4); - mService.mPendingJobs.add(job); - } - - verifyPendingJobComparatorTransitivity(); - } - } - - @Test - @LargeTest - public void testPendingJobSortingTransitivity_Concentrated_WithPriority() { - // 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) { - final boolean isEj = random.nextFloat() < .03; - final int priority; - if (isEj) { - priority = random.nextBoolean() ? JobInfo.PRIORITY_MAX : JobInfo.PRIORITY_HIGH; - } else { - priority = sRegJobPriorities[random.nextInt(sRegJobPriorities.length)]; - } - JobStatus job = createJobStatus( - "testPendingJobSortingTransitivity_Concentrated_WithPriority", - createJobInfo(i).setExpedited(isEj).setPriority(priority), - 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) { - 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); - } - } - } - } - } }