audiohal: Make closing of effects and streams fast

There were two problems:

1. Joining of reader / writer / process threads (the threads that
   interact with HAL) was taking up to 1 second because the thread
   was usually waiting for an event flag to be toggled, or a 1s
   timeout.

2. Calling IStream.close or IEffect.close shouldn't tax the caller.
   Changed the code so a call to close only signals the thread that
   it's time to exit, and then the thread is only joined in the
   effect or stream destructor.

Bug: 34800063
Bug: 34499806
Test: see repro steps in the bugs
Change-Id: Ife20524a1eba4ec9a78152e89862526e8cd5c960

diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp
index ad18986..b444423 100644 (file)
--- a/audio/2.0/default/StreamIn.cpp
+++ b/audio/2.0/default/StreamIn.cpp
@@ -16,10 +16,12 @@
 
 #define LOG_TAG "StreamInHAL"
 //#define LOG_NDEBUG 0
+#define ATRACE_TAG ATRACE_TAG_AUDIO
 
 #include <android/log.h>
 #include <hardware/audio.h>
 #include <mediautils/SchedulingPolicyService.h>
+#include <utils/Trace.h>
 
 #include "StreamIn.h"
 
@@ -120,7 +122,18 @@ StreamIn::StreamIn(audio_hw_device_t* device, audio_stream_in_t* stream)
 }
 
 StreamIn::~StreamIn() {
+    ATRACE_CALL();
     close();
+    if (mReadThread.get()) {
+        ATRACE_NAME("mReadThread->join");
+        status_t status = mReadThread->join();
+        ALOGE_IF(status, "read thread exit error: %s", strerror(-status));
+    }
+    if (mEfGroup) {
+        status_t status = EventFlag::deleteEventFlag(&mEfGroup);
+        ALOGE_IF(status, "read MQ event flag deletion error: %s", strerror(-status));
+    }
+    mDevice->close_input_stream(mDevice, mStream);
     mStream = nullptr;
     mDevice = nullptr;
 }
@@ -240,14 +253,10 @@ Return<Result> StreamIn::close()  {
     mIsClosed = true;
     if (mReadThread.get()) {
         mStopReadThread.store(true, std::memory_order_release);
-        status_t status = mReadThread->requestExitAndWait();
-        ALOGE_IF(status, "read thread exit error: %s", strerror(-status));
     }
     if (mEfGroup) {
-        status_t status = EventFlag::deleteEventFlag(&mEfGroup);
-        ALOGE_IF(status, "read MQ event flag deletion error: %s", strerror(-status));
+        mEfGroup->wake(static_cast<uint32_t>(MessageQueueFlagBits::NOT_FULL));
     }
-    mDevice->close_input_stream(mDevice, mStream);
     return Result::OK;
 }
 

diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp
index 1948b68..bfd592c 100644 (file)
--- a/audio/2.0/default/StreamOut.cpp
+++ b/audio/2.0/default/StreamOut.cpp
@@ -16,10 +16,12 @@
 
 #define LOG_TAG "StreamOutHAL"
 //#define LOG_NDEBUG 0
+#define ATRACE_TAG ATRACE_TAG_AUDIO
 
 #include <android/log.h>
 #include <hardware/audio.h>
 #include <mediautils/SchedulingPolicyService.h>
+#include <utils/Trace.h>
 
 #include "StreamOut.h"
 
@@ -121,7 +123,19 @@ StreamOut::StreamOut(audio_hw_device_t* device, audio_stream_out_t* stream)
 }
 
 StreamOut::~StreamOut() {
+    ATRACE_CALL();
     close();
+    if (mWriteThread.get()) {
+        ATRACE_NAME("mWriteThread->join");
+        status_t status = mWriteThread->join();
+        ALOGE_IF(status, "write thread exit error: %s", strerror(-status));
+    }
+    if (mEfGroup) {
+        status_t status = EventFlag::deleteEventFlag(&mEfGroup);
+        ALOGE_IF(status, "write MQ event flag deletion error: %s", strerror(-status));
+    }
+    mCallback.clear();
+    mDevice->close_output_stream(mDevice, mStream);
     mStream = nullptr;
     mDevice = nullptr;
 }
@@ -225,15 +239,10 @@ Return<Result> StreamOut::close()  {
     mIsClosed = true;
     if (mWriteThread.get()) {
         mStopWriteThread.store(true, std::memory_order_release);
-        status_t status = mWriteThread->requestExitAndWait();
-        ALOGE_IF(status, "write thread exit error: %s", strerror(-status));
     }
     if (mEfGroup) {
-        status_t status = EventFlag::deleteEventFlag(&mEfGroup);
-        ALOGE_IF(status, "write MQ event flag deletion error: %s", strerror(-status));
+        mEfGroup->wake(static_cast<uint32_t>(MessageQueueFlagBits::NOT_EMPTY));
     }
-    mCallback.clear();
-    mDevice->close_output_stream(mDevice, mStream);
     return Result::OK;
 }
 

diff --git a/audio/effect/2.0/default/Effect.cpp b/audio/effect/2.0/default/Effect.cpp
index 3c97fc4..0a4aeb7 100644 (file)
--- a/audio/effect/2.0/default/Effect.cpp
+++ b/audio/effect/2.0/default/Effect.cpp
@@ -17,8 +17,11 @@
 #include <memory.h>
 
 #define LOG_TAG "EffectHAL"
+#define ATRACE_TAG ATRACE_TAG_AUDIO
+
 #include <android/log.h>
 #include <media/EffectsFactoryApi.h>
+#include <utils/Trace.h>
 
 #include "Conversions.h"
 #include "Effect.h"
@@ -78,8 +81,9 @@ bool ProcessThread::threadLoop() {
                 static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_PROCESS_ALL),
                 &efState,
                 NS_PER_SEC);
-        if (!(efState & static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_PROCESS_ALL))) {
-            continue;  // Nothing to do.
+        if (!(efState & static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_PROCESS_ALL))
+                || (efState & static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_QUIT))) {
+            continue;  // Nothing to do or time to quit.
         }
         Result retval = Result::OK;
         if (efState & static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_PROCESS_REVERSE)
@@ -134,7 +138,23 @@ Effect::Effect(effect_handle_t handle)
 }
 
 Effect::~Effect() {
+    ATRACE_CALL();
     close();
+    if (mProcessThread.get()) {
+        ATRACE_NAME("mProcessThread->join");
+        status_t status = mProcessThread->join();
+        ALOGE_IF(status, "processing thread exit error: %s", strerror(-status));
+    }
+    if (mEfGroup) {
+        status_t status = EventFlag::deleteEventFlag(&mEfGroup);
+        ALOGE_IF(status, "processing MQ event flag deletion error: %s", strerror(-status));
+    }
+    mInBuffer.clear();
+    mOutBuffer.clear();
+    int status = EffectRelease(mHandle);
+    ALOGW_IF(status, "Error releasing effect %p: %s", mHandle, strerror(-status));
+    EffectMap::getInstance().remove(mHandle);
+    mHandle = 0;
 }
 
 // static
@@ -731,19 +751,10 @@ Return<Result> Effect::close() {
     mIsClosed = true;
     if (mProcessThread.get()) {
         mStopProcessThread.store(true, std::memory_order_release);
-        status_t status = mProcessThread->requestExitAndWait();
-        ALOGE_IF(status, "processing thread exit error: %s", strerror(-status));
     }
     if (mEfGroup) {
-        status_t status = EventFlag::deleteEventFlag(&mEfGroup);
-        ALOGE_IF(status, "processing MQ event flag deletion error: %s", strerror(-status));
+        mEfGroup->wake(static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_QUIT));
     }
-    mInBuffer.clear();
-    mOutBuffer.clear();
-    int status = EffectRelease(mHandle);
-    ALOGW_IF(status, "Error releasing effect %p: %s", mHandle, strerror(-status));
-    EffectMap::getInstance().remove(mHandle);
-    mHandle = 0;
     return Result::OK;
 }
 

diff --git a/audio/effect/2.0/types.hal b/audio/effect/2.0/types.hal
index 0cac59a..0626ec5 100644 (file)
--- a/audio/effect/2.0/types.hal
+++ b/audio/effect/2.0/types.hal
@@ -293,5 +293,7 @@ enum MessageQueueFlagBits : uint32_t {
     DONE_PROCESSING = 1 << 0,
     REQUEST_PROCESS = 1 << 1,
     REQUEST_PROCESS_REVERSE = 1 << 2,
-    REQUEST_PROCESS_ALL = REQUEST_PROCESS | REQUEST_PROCESS_REVERSE
+    REQUEST_QUIT = 1 << 3,
+    REQUEST_PROCESS_ALL =
+        REQUEST_PROCESS | REQUEST_PROCESS_REVERSE | REQUEST_QUIT
 };