Merge "Try to avoid overlapping job execution." into tm-dev

5 files changed, 93 insertions, 11 deletions

diff --git a/apex/jobscheduler/framework/java/android/app/job/JobInfo.java b/apex/jobscheduler/framework/java/android/app/job/JobInfo.java
index f49cdbf403f0..4710322db283 100644
--- a/apex/jobscheduler/framework/java/android/app/job/JobInfo.java
+++ b/apex/jobscheduler/framework/java/android/app/job/JobInfo.java
@@ -1777,7 +1777,10 @@ public class JobInfo implements Parcelable {
          * {@link Build.VERSION_CODES#S}, but starting from Android version
          * {@link Build.VERSION_CODES#TIRAMISU}, expedited jobs for the foreground app are
          * guaranteed to be started before {@link JobScheduler#schedule(JobInfo)} returns (assuming
-         * all requested constraints are satisfied), similar to foreground services.
+         * all requested constraints are satisfied), similar to foreground services. However, this
+         * start guarantee means there is a higher chance of overlapping executions, as noted in
+         * {@link JobService}, so be sure to handle that properly if you intend to reschedule the
+         * job while it's actively running.
          *
          * @see JobInfo#isExpedited()
          */
diff --git a/apex/jobscheduler/framework/java/android/app/job/JobScheduler.java b/apex/jobscheduler/framework/java/android/app/job/JobScheduler.java
index 1f4ef0470ebd..388bbf1b26a0 100644
--- a/apex/jobscheduler/framework/java/android/app/job/JobScheduler.java
+++ b/apex/jobscheduler/framework/java/android/app/job/JobScheduler.java
@@ -107,7 +107,9 @@ public abstract class JobScheduler {
     /**
      * Schedule a job to be executed.  Will replace any currently scheduled job with the same
      * ID with the new information in the {@link JobInfo}.  If a job with the given ID is currently
-     * running, it will be stopped.
+     * running, it will be stopped. Note that in some cases, the newly scheduled job may be started
+     * before the previously running job has been fully stopped. See {@link JobService} for
+     * additional details.
      *
      * <p class="caution"><strong>Note:</strong> Scheduling a job can have a high cost, even if it's
      * rescheduling the same job and the job didn't execute, especially on platform versions before
@@ -131,7 +133,7 @@ public abstract class JobScheduler {
      * job.  If a job with the same ID is already scheduled, it will be replaced with the
      * new {@link JobInfo}, but any previously enqueued work will remain and be dispatched the
      * next time it runs.  If a job with the same ID is already running, the new work will be
-     * enqueued for it.
+     * enqueued for it without stopping the job.
      *
      * <p>The work you enqueue is later retrieved through
      * {@link JobParameters#dequeueWork() JobParameters.dequeueWork}.  Be sure to see there
diff --git a/apex/jobscheduler/framework/java/android/app/job/JobService.java b/apex/jobscheduler/framework/java/android/app/job/JobService.java
index e5b07429a5c6..7ed4b62ae7e4 100644
--- a/apex/jobscheduler/framework/java/android/app/job/JobService.java
+++ b/apex/jobscheduler/framework/java/android/app/job/JobService.java
@@ -31,6 +31,17 @@ import android.os.IBinder;
  * in blocking any future callbacks from the JobManager - specifically
  * {@link #onStopJob(android.app.job.JobParameters)}, which is meant to inform you that the
  * scheduling requirements are no longer being met.</p>
+ *
+ * As a subclass of {@link Service}, there will only be one active instance of any JobService
+ * subclasses, regardless of job ID. This means that if you schedule multiple jobs with different
+ * job IDs but using the same JobService class, that JobService may receive multiple calls to
+ * {@link #onStartJob(JobParameters)} and {@link #onStopJob(JobParameters)}, with each call being
+ * for the separate jobs.
+ *
+ * <p class="note">Note that if you cancel and reschedule an already executing job,
+ * there may be a small period of time where {@link #onStartJob(JobParameters)} has been called for
+ * the newly scheduled job instance before {@link #onStopJob(JobParameters)} has been called or
+ * fully processed for the old job.</p>
  */
 public abstract class JobService extends Service {
     private static final String TAG = "JobService";
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 c86353c84467..afe36b5fa25a 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java
@@ -542,6 +542,22 @@ class JobConcurrencyManager {
         return mRunningJobs.contains(job);
     }
 
+    /**
+     * Returns true if a job that is "similar" to the provided job is currently running.
+     * "Similar" in this context means any job that the {@link JobStore} would consider equivalent
+     * and replace one with the other.
+     */
+    @GuardedBy("mLock")
+    private boolean isSimilarJobRunningLocked(JobStatus job) {
+        for (int i = mRunningJobs.size() - 1; i >= 0; --i) {
+            JobStatus js = mRunningJobs.valueAt(i);
+            if (job.getUid() == js.getUid() && job.getJobId() == js.getJobId()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /** Return {@code true} if the state was updated. */
     @GuardedBy("mLock")
     private boolean refreshSystemStateLocked() {
@@ -699,6 +715,21 @@ class JobConcurrencyManager {
                 continue;
             }
 
+            final boolean isTopEj = nextPending.shouldTreatAsExpeditedJob()
+                    && nextPending.lastEvaluatedBias == JobInfo.BIAS_TOP_APP;
+            // Avoid overlapping job execution as much as possible.
+            if (!isTopEj && isSimilarJobRunningLocked(nextPending)) {
+                if (DEBUG) {
+                    Slog.w(TAG, "Delaying execution of job because of similarly running one: "
+                            + nextPending);
+                }
+                // It would be nice to let the JobService running the other similar job know about
+                // this new job so that it doesn't unbind from the JobService and we can call
+                // onStartJob as soon as the older job finishes.
+                // TODO: optimize the job reschedule flow to reduce service binding churn
+                continue;
+            }
+
             // Find an available slot for nextPending. The context should be one of the following:
             // 1. Unused
             // 2. Its job should have used up its minimum execution guarantee so it
@@ -707,8 +738,6 @@ class JobConcurrencyManager {
             ContextAssignment selectedContext = null;
             final int allWorkTypes = getJobWorkTypes(nextPending);
             final boolean pkgConcurrencyOkay = !isPkgConcurrencyLimitedLocked(nextPending);
-            final boolean isTopEj = nextPending.shouldTreatAsExpeditedJob()
-                    && nextPending.lastEvaluatedBias == JobInfo.BIAS_TOP_APP;
             final boolean isInOverage = projectedRunningCount > STANDARD_CONCURRENCY_LIMIT;
             boolean startingJob = false;
             if (idle.size() > 0) {
@@ -1177,6 +1206,15 @@ class JobConcurrencyManager {
                     continue;
                 }
 
+                // Avoid overlapping job execution as much as possible.
+                if (isSimilarJobRunningLocked(nextPending)) {
+                    if (DEBUG) {
+                        Slog.w(TAG, "Avoiding execution of job because of similarly running one: "
+                                + nextPending);
+                    }
+                    continue;
+                }
+
                 if (worker.getPreferredUid() != nextPending.getUid()) {
                     if (backupJob == null && !isPkgConcurrencyLimitedLocked(nextPending)) {
                         int allWorkTypes = getJobWorkTypes(nextPending);
@@ -1260,6 +1298,15 @@ class JobConcurrencyManager {
                     continue;
                 }
 
+                // Avoid overlapping job execution as much as possible.
+                if (isSimilarJobRunningLocked(nextPending)) {
+                    if (DEBUG) {
+                        Slog.w(TAG, "Avoiding execution of job because of similarly running one: "
+                                + nextPending);
+                    }
+                    continue;
+                }
+
                 if (isPkgConcurrencyLimitedLocked(nextPending)) {
                     continue;
                 }
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 358c32722b8b..cd70e88b18aa 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
@@ -1208,12 +1208,22 @@ public class JobSchedulerService extends com.android.server.SystemService
             // This may throw a SecurityException.
             jobStatus.prepareLocked();
 
+            final boolean canExecuteImmediately;
             if (toCancel != null) {
                 // Implicitly replaces the existing job record with the new instance
-                cancelJobImplLocked(toCancel, jobStatus, JobParameters.STOP_REASON_CANCELLED_BY_APP,
-                        JobParameters.INTERNAL_STOP_REASON_CANCELED, "job rescheduled by app");
+                final boolean wasJobExecuting = cancelJobImplLocked(toCancel, jobStatus,
+                        JobParameters.STOP_REASON_CANCELLED_BY_APP,
+                        JobParameters.INTERNAL_STOP_REASON_CANCELED,
+                        "job rescheduled by app");
+                // Avoid overlapping job executions. Don't push for immediate execution if an old
+                // job with the same ID was running, but let TOP EJs start immediately.
+                canExecuteImmediately = !wasJobExecuting
+                        || (jobStatus.isRequestedExpeditedJob()
+                        && mUidBiasOverride.get(jobStatus.getSourceUid(), JobInfo.BIAS_DEFAULT)
+                        == JobInfo.BIAS_TOP_APP);
             } else {
                 startTrackingJobLocked(jobStatus, null);
+                canExecuteImmediately = true;
             }
 
             if (work != null) {
@@ -1256,7 +1266,12 @@ public class JobSchedulerService extends com.android.server.SystemService
                 // list and try to run it.
                 mJobPackageTracker.notePending(jobStatus);
                 mPendingJobQueue.add(jobStatus);
-                maybeRunPendingJobsLocked();
+                if (canExecuteImmediately) {
+                    // Don't ask the JobConcurrencyManager to try to run the job immediately. The
+                    // JobServiceContext will ask the JobConcurrencyManager for another job once
+                    // it finishes cleaning up the old job.
+                    maybeRunPendingJobsLocked();
+                }
             } else {
                 evaluateControllerStatesLocked(jobStatus);
             }
@@ -1377,8 +1392,10 @@ public class JobSchedulerService extends com.android.server.SystemService
      * is null, the cancelled job is removed outright from the system.  If
      * {@code incomingJob} is non-null, it replaces {@code cancelled} in the store of
      * currently scheduled jobs.
+     *
+     * @return true if the cancelled job was running
      */
-    private void cancelJobImplLocked(JobStatus cancelled, JobStatus incomingJob,
+    private boolean cancelJobImplLocked(JobStatus cancelled, JobStatus incomingJob,
             @JobParameters.StopReason int reason, int internalReasonCode, String debugReason) {
         if (DEBUG) Slog.d(TAG, "CANCEL: " + cancelled.toShortString());
         cancelled.unprepareLocked();
@@ -1389,7 +1406,7 @@ public class JobSchedulerService extends com.android.server.SystemService
         }
         mChangedJobList.remove(cancelled);
         // Cancel if running.
-        mConcurrencyManager.stopJobOnServiceContextLocked(
+        boolean wasRunning = mConcurrencyManager.stopJobOnServiceContextLocked(
                 cancelled, reason, internalReasonCode, debugReason);
         // If this is a replacement, bring in the new version of the job
         if (incomingJob != null) {
@@ -1397,6 +1414,7 @@ public class JobSchedulerService extends com.android.server.SystemService
             startTrackingJobLocked(incomingJob, cancelled);
         }
         reportActiveLocked();
+        return wasRunning;
     }
 
     void updateUidState(int uid, int procState) {
@@ -1755,7 +1773,7 @@ public class JobSchedulerService extends com.android.server.SystemService
             // same job ID), we remove it from the JobStore and tell the JobServiceContext to stop
             // running the job. Once the job stops running, we then call this method again.
             // TODO: rework code so we don't intentionally call this method twice for the same job
-            Slog.w(TAG, "Job didn't exist in JobStore");
+            Slog.w(TAG, "Job didn't exist in JobStore: " + jobStatus.toShortString());
         }
         if (mReadyToRock) {
             for (int i = 0; i < mControllers.size(); i++) {
@@ -3813,6 +3831,7 @@ public class JobSchedulerService extends com.android.server.SystemService
                     // Double indent for readability
                     pw.increaseIndent();
                     pw.increaseIndent();
+                    pw.println(job.toShortString());
                     job.dump(pw, true, nowElapsed);
                     pw.decreaseIndent();
                     pw.decreaseIndent();