commit 8501ee9789bb9053002556ec163902cb5780c573
author Yu-Han Yang <yuhany@google.com> Wed Mar 19 15:59:46 2025 +0000
committer Yu-Han Yang <yuhany@google.com> Thu Mar 20 16:00:59 2025 -0700
tree 0f99359052a7612c92fac09c4940e20148be8a9b
parent 8b12bc80203c89e11402bd56a7051a29d185ff8b

Move thread creation to setCallback() from start()

This change only modifies the Cuttlefish emulator behavior.

Creating a new thread is a heavy operation. Doing so in start() slows
down the request handling. In a stress test like
testVariedRatesRepetitive, 100+ requests are received in a short time,
which results in accumulated requests waiting to be handled, and thus
breaks the subsequent tests.

This change moves creating a new thread from start() to setCallback()
to avoid thread creation for each new request.

Bug: 400325468
Test: atest CtsLocationGnssTestCases
Test: atest GtsLocationTestCases
Test: atest VtsHalGnssTargetTest
Change-Id: I6bf89989c572b0e7b1b9ba9339b922363ef26ce4

gnss/aidl/default/Gnss.cpp

diff --git a/gnss/aidl/default/Gnss.cpp b/gnss/aidl/default/Gnss.cpp
index 1fd21d8..a0510ce 100644
--- a/gnss/aidl/default/Gnss.cpp
+++ b/gnss/aidl/default/Gnss.cpp
@@ -91,6 +91,52 @@
     if (!status.isOk()) {
         ALOGE("%s: Unable to invoke callback.gnssSetSignalTypeCapabilitiesCb", __func__);
     }
+
+    // In case when the setCallback() and close() calls are not balanced
+    mIsInitialized = false;
+    mIsActive = false;
+    mThreadBlocker.notify();
+    if (mThread.joinable()) {
+        mThread.join();
+    }
+
+    mIsInitialized = true;
+    mThreadBlocker.reset();
+    mThread = std::thread([this]() {
+        while (mIsInitialized) {
+            if (mIsActive) {
+                if (mReportedLocationCount == 0) {
+                    if (!mGnssMeasurementEnabled || mMinIntervalMs <= mGnssMeasurementIntervalMs) {
+                        this->reportSvStatus();
+                    }
+                    if (!mFirstFixReceived) {
+                        // Simulate the code start TTFF
+                        std::this_thread::sleep_for(std::chrono::milliseconds(TTFF_MILLIS));
+                        mFirstFixReceived = true;
+                    }
+                }
+                if (!mGnssMeasurementEnabled || mMinIntervalMs <= mGnssMeasurementIntervalMs) {
+                    this->reportSvStatus();
+                }
+                this->reportNmea();
+
+                auto currentLocation = getLocationFromHW();
+                mGnssPowerIndication->notePowerConsumption();
+                if (currentLocation != nullptr) {
+                    this->reportLocation(*currentLocation);
+                } else {
+                    const auto location = Utils::getMockLocation();
+                    this->reportLocation(location);
+                }
+                mReportedLocationCount += 1;
+                mThreadBlocker.wait_for(std::chrono::milliseconds(mMinIntervalMs));
+            } else {
+                // Wait indefinitely until start() or close() is called
+                mThreadBlocker.wait();
+            }
+        }
+    });
+
     return ScopedAStatus::ok();
 }
 
@@ -104,48 +150,16 @@
 }
 
 ScopedAStatus Gnss::start() {
-    ALOGD("start()");
+    ALOGD("start");
     if (mIsActive) {
         ALOGW("Gnss has started. Restarting...");
         stop();
     }
-
     mIsActive = true;
-    mThreadBlocker.reset();
     // notify measurement engine to update measurement interval
     mGnssMeasurementInterface->setLocationEnabled(true);
     this->reportGnssStatusValue(IGnssCallback::GnssStatusValue::SESSION_BEGIN);
-    mThread = std::thread([this]() {
-        if (!mGnssMeasurementEnabled || mMinIntervalMs <= mGnssMeasurementIntervalMs) {
-            this->reportSvStatus();
-        }
-        if (!mFirstFixReceived) {
-            std::this_thread::sleep_for(std::chrono::milliseconds(TTFF_MILLIS));
-            mFirstFixReceived = true;
-        }
-        int reportGnssCount = 0;
-        do {
-            if (!mIsActive) {
-                ALOGD("Do not report location. mIsActive is false");
-                break;
-            }
-            reportGnssCount += 1;
-            if (!mGnssMeasurementEnabled || mMinIntervalMs <= mGnssMeasurementIntervalMs) {
-                this->reportSvStatus();
-            }
-            this->reportNmea();
-
-            auto currentLocation = getLocationFromHW();
-            mGnssPowerIndication->notePowerConsumption();
-            if (currentLocation != nullptr) {
-                this->reportLocation(*currentLocation);
-            } else {
-                const auto location = Utils::getMockLocation();
-                this->reportLocation(location);
-            }
-        } while (mIsActive && mThreadBlocker.wait_for(std::chrono::milliseconds(mMinIntervalMs)));
-        ALOGD("reportGnssCount: %d", reportGnssCount);
-    });
+    mThreadBlocker.notify();
     return ScopedAStatus::ok();
 }
 
@@ -154,16 +168,22 @@
     mIsActive = false;
     mGnssMeasurementInterface->setLocationEnabled(false);
     this->reportGnssStatusValue(IGnssCallback::GnssStatusValue::SESSION_END);
+
+    int reportedLocationCount = mReportedLocationCount;
+    ALOGD("reportedLocationCount: %d", reportedLocationCount);
+    mReportedLocationCount = 0;
     mThreadBlocker.notify();
-    if (mThread.joinable()) {
-        mThread.join();
-    }
     return ScopedAStatus::ok();
 }
 
 ScopedAStatus Gnss::close() {
     ALOGD("close");
     sGnssCallback = nullptr;
+    mIsInitialized = false;
+    mThreadBlocker.notify();
+    if (mThread.joinable()) {
+        mThread.join();
+    }
     return ScopedAStatus::ok();
 }