From 5e204c1fe2a2321df4377d7d6542755e00b6ab8c Mon Sep 17 00:00:00 2001
From: Kweku Adams <kwekua@google.com>
Date: Wed, 10 May 2023 20:26:42 +0000
Subject: [PATCH] Fix concurrency logging.

Avoid overcounting lower concurrency values from jobs finishing
execution.

Bug: 138239687
Bug: 279935506
Test: atest CtsJobSchedulerTestCases:JobSchedulingTest
Change-Id: I000dcfba72d8394486889dbb53aaa7039c62fac3
---
 .../server/job/JobConcurrencyManager.java     | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 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 a0634f0e74eb0..dc608e7fddfdc 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java
@@ -810,7 +810,7 @@ class JobConcurrencyManager {
                 mRecycledChanged, mRecycledIdle, mRecycledPreferredUidOnly, mRecycledStoppable,
                 mRecycledAssignmentInfo, mRecycledPrivilegedState);
 
-        noteConcurrency();
+        noteConcurrency(true);
     }
 
     @VisibleForTesting
@@ -1437,11 +1437,13 @@ class JobConcurrencyManager {
         }
     }
 
-    private void noteConcurrency() {
+    private void noteConcurrency(boolean logForHistogram) {
         mService.mJobPackageTracker.noteConcurrency(mRunningJobs.size(),
                 // TODO: log per type instead of only TOP
                 mWorkCountTracker.getRunningJobCount(WORK_TYPE_TOP));
-        sConcurrencyHistogramLogger.logSample(mActiveServices.size());
+        if (logForHistogram) {
+            sConcurrencyHistogramLogger.logSample(mActiveServices.size());
+        }
     }
 
     @GuardedBy("mLock")
@@ -1582,7 +1584,9 @@ class JobConcurrencyManager {
         final PendingJobQueue pendingJobQueue = mService.getPendingJobQueue();
         if (pendingJobQueue.size() == 0) {
             worker.clearPreferredUid();
-            noteConcurrency();
+            // Don't log the drop in concurrency to the histogram, otherwise, we'll end up
+            // overcounting lower concurrency values as jobs end execution.
+            noteConcurrency(false);
             return;
         }
         if (mActiveServices.size() >= mSteadyStateConcurrencyLimit) {
@@ -1612,7 +1616,9 @@ class JobConcurrencyManager {
                 // scheduled), but we should
                 // be able to stop the other jobs soon so don't start running anything new until we
                 // get back below the limit.
-                noteConcurrency();
+                // Don't log the drop in concurrency to the histogram, otherwise, we'll end up
+                // overcounting lower concurrency values as jobs end execution.
+                noteConcurrency(false);
                 return;
             }
         }
@@ -1761,7 +1767,9 @@ class JobConcurrencyManager {
             }
         }
 
-        noteConcurrency();
+        // Don't log the drop in concurrency to the histogram, otherwise, we'll end up
+        // overcounting lower concurrency values as jobs end execution.
+        noteConcurrency(false);
     }
 
     /**