From bb5e3b5f656bda4041858ab2c54b7adb28f5239d Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Wed, 6 Feb 2019 18:10:11 -0800 Subject: [PATCH] Reducing overhead of job constraint evaluation. 1. TimeController was checking its expired deadline and delay jobs even when the job in question already had its constraint satisfied. 2. In the worst case, TimeController.evaluateStateLocked would check all N jobs even though the job in question is the only one that matters in that case. Adding the wouldBeReadyWithConstraint check in evaluateStateLocked reduces worst case runtime by a factor of N. 3. With the introduction of job alarm skipping, TimeController would sometimes schedule an alarm with AlarmManager for the exact same time it had the alarm set for. Adding a check removes that unnecessary call. Bug: 123756120 Bug: 123879683 Test: atest com.android.server.job.controllers.JobStatusTest Test: atest com.android.server.job.controllers.TimeControllerTest Test: atest FrameworksMockingServicesTests Test: atest JobThrottlingTest Test: atest TimingConstraintsTest Change-Id: Iedf7873398a6938f6dd4fed5e92ee2e8d2c211bb --- .../server/job/JobSchedulerService.java | 2 +- .../job/controllers/StateController.java | 1 + .../job/controllers/TimeController.java | 59 +++++++++++++++---- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/job/JobSchedulerService.java b/services/core/java/com/android/server/job/JobSchedulerService.java index 19d10ecfb34d8..cefe583fc5d18 100644 --- a/services/core/java/com/android/server/job/JobSchedulerService.java +++ b/services/core/java/com/android/server/job/JobSchedulerService.java @@ -509,7 +509,7 @@ public class JobSchedulerService extends com.android.server.SystemService private static final float DEFAULT_CONN_CONGESTION_DELAY_FRAC = 0.5f; private static final float DEFAULT_CONN_PREFETCH_RELAX_FRAC = 0.5f; private static final boolean DEFAULT_USE_HEARTBEATS = false; - private static final boolean DEFAULT_TIME_CONTROLLER_SKIP_NOT_READY_JOBS = false; + private static final boolean DEFAULT_TIME_CONTROLLER_SKIP_NOT_READY_JOBS = true; private static final long DEFAULT_QUOTA_CONTROLLER_ALLOWED_TIME_PER_PERIOD_MS = 10 * 60 * 1000L; // 10 minutes private static final long DEFAULT_QUOTA_CONTROLLER_IN_QUOTA_BUFFER_MS = diff --git a/services/core/java/com/android/server/job/controllers/StateController.java b/services/core/java/com/android/server/job/controllers/StateController.java index 97c3bac4ddbb9..74628fb9b502b 100644 --- a/services/core/java/com/android/server/job/controllers/StateController.java +++ b/services/core/java/com/android/server/job/controllers/StateController.java @@ -110,6 +110,7 @@ public abstract class StateController { final boolean jobWouldBeReady = jobStatus.wouldBeReadyWithConstraint(constraint); if (DEBUG) { Slog.v(TAG, "wouldBeReadyWithConstraintLocked: " + jobStatus.toShortString() + + " constraint=" + constraint + " readyWithConstraint=" + jobWouldBeReady); } if (!jobWouldBeReady) { diff --git a/services/core/java/com/android/server/job/controllers/TimeController.java b/services/core/java/com/android/server/job/controllers/TimeController.java index 26f3caf2e4872..70deb38080b4a 100644 --- a/services/core/java/com/android/server/job/controllers/TimeController.java +++ b/services/core/java/com/android/server/job/controllers/TimeController.java @@ -149,8 +149,8 @@ public final class TimeController extends StateController { @Override public void onConstantsUpdatedLocked() { - checkExpiredDelaysAndResetAlarm(); checkExpiredDeadlinesAndResetAlarm(); + checkExpiredDelaysAndResetAlarm(); } @Override @@ -159,20 +159,47 @@ public final class TimeController extends StateController { return; } - if (job.hasTimingDelayConstraint() - && job.getEarliestRunTime() <= mNextDelayExpiredElapsedMillis) { - checkExpiredDelaysAndResetAlarm(); - } + final long nowElapsedMillis = sElapsedRealtimeClock.millis(); + + // Check deadline constraint first because if it's satisfied, we avoid a little bit of + // unnecessary processing of the timing delay. if (job.hasDeadlineConstraint() + && !job.isConstraintSatisfied(JobStatus.CONSTRAINT_DEADLINE) && job.getLatestRunTimeElapsed() <= mNextJobExpiredElapsedMillis) { - checkExpiredDeadlinesAndResetAlarm(); + if (evaluateDeadlineConstraint(job, nowElapsedMillis)) { + checkExpiredDeadlinesAndResetAlarm(); + checkExpiredDelaysAndResetAlarm(); + } else { + final boolean isAlarmForJob = + job.getLatestRunTimeElapsed() == mNextJobExpiredElapsedMillis; + final boolean wouldBeReady = wouldBeReadyWithConstraintLocked( + job, JobStatus.CONSTRAINT_DEADLINE); + if ((isAlarmForJob && !wouldBeReady) || (!isAlarmForJob && wouldBeReady)) { + checkExpiredDeadlinesAndResetAlarm(); + } + } + } + if (job.hasTimingDelayConstraint() + && !job.isConstraintSatisfied(JobStatus.CONSTRAINT_TIMING_DELAY) + && job.getEarliestRunTime() <= mNextDelayExpiredElapsedMillis) { + if (evaluateTimingDelayConstraint(job, nowElapsedMillis)) { + checkExpiredDelaysAndResetAlarm(); + } else { + final boolean isAlarmForJob = + job.getEarliestRunTime() == mNextDelayExpiredElapsedMillis; + final boolean wouldBeReady = wouldBeReadyWithConstraintLocked( + job, JobStatus.CONSTRAINT_TIMING_DELAY); + if ((isAlarmForJob && !wouldBeReady) || (!isAlarmForJob && wouldBeReady)) { + checkExpiredDelaysAndResetAlarm(); + } + } } } @Override public void reevaluateStateLocked(int uid) { - checkExpiredDelaysAndResetAlarm(); checkExpiredDeadlinesAndResetAlarm(); + checkExpiredDelaysAndResetAlarm(); } /** @@ -182,10 +209,10 @@ public final class TimeController extends StateController { * back and forth. */ private boolean canStopTrackingJobLocked(JobStatus job) { - return (!job.hasTimingDelayConstraint() || - (job.satisfiedConstraints&JobStatus.CONSTRAINT_TIMING_DELAY) != 0) && - (!job.hasDeadlineConstraint() || - (job.satisfiedConstraints&JobStatus.CONSTRAINT_DEADLINE) != 0); + return (!job.hasTimingDelayConstraint() + || job.isConstraintSatisfied(JobStatus.CONSTRAINT_TIMING_DELAY)) + && (!job.hasDeadlineConstraint() + || job.isConstraintSatisfied(JobStatus.CONSTRAINT_DEADLINE)); } private void ensureAlarmServiceLocked() { @@ -241,6 +268,7 @@ public final class TimeController extends StateController { } } + /** @return true if the job's deadline constraint is satisfied */ private boolean evaluateDeadlineConstraint(JobStatus job, long nowElapsedMillis) { final long jobDeadline = job.getLatestRunTimeElapsed(); @@ -279,7 +307,7 @@ public final class TimeController extends StateController { if (job.isReady()) { ready = true; } - } else if (!job.isConstraintSatisfied(JobStatus.CONSTRAINT_TIMING_DELAY)) { + } else { if (mConstants.TIME_CONTROLLER_SKIP_NOT_READY_JOBS && !wouldBeReadyWithConstraintLocked( job, JobStatus.CONSTRAINT_TIMING_DELAY)) { @@ -319,6 +347,7 @@ public final class TimeController extends StateController { } } + /** @return true if the job's delay constraint is satisfied */ private boolean evaluateTimingDelayConstraint(JobStatus job, long nowElapsedMillis) { final long jobDelayTime = job.getEarliestRunTime(); if (jobDelayTime <= nowElapsedMillis) { @@ -347,6 +376,9 @@ public final class TimeController extends StateController { */ private void setDelayExpiredAlarmLocked(long alarmTimeElapsedMillis, WorkSource ws) { alarmTimeElapsedMillis = maybeAdjustAlarmTime(alarmTimeElapsedMillis); + if (mNextDelayExpiredElapsedMillis == alarmTimeElapsedMillis) { + return; + } mNextDelayExpiredElapsedMillis = alarmTimeElapsedMillis; updateAlarmWithListenerLocked(DELAY_TAG, mNextDelayExpiredListener, mNextDelayExpiredElapsedMillis, ws); @@ -359,6 +391,9 @@ public final class TimeController extends StateController { */ private void setDeadlineExpiredAlarmLocked(long alarmTimeElapsedMillis, WorkSource ws) { alarmTimeElapsedMillis = maybeAdjustAlarmTime(alarmTimeElapsedMillis); + if (mNextJobExpiredElapsedMillis == alarmTimeElapsedMillis) { + return; + } mNextJobExpiredElapsedMillis = alarmTimeElapsedMillis; updateAlarmWithListenerLocked(DEADLINE_TAG, mDeadlineExpiredListener, mNextJobExpiredElapsedMillis, ws);