From 4f5d6801e9da3e728d87235a496a520bea224c68 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Mon, 22 Feb 2021 13:10:55 -0800 Subject: [PATCH] Add move-only lambdas support Bug: 166800618 Test: atest --host android.hardware.biometrics.fingerprint.WorkerThreadTest Change-Id: I582d44d5098b7426663b75200c822bc6e8bb70a6 --- .../fingerprint/aidl/default/WorkerThread.cpp | 6 +-- .../aidl/default/include/Callable.h | 54 +++++++++++++++++++ .../aidl/default/include/WorkerThread.h | 13 +++-- .../aidl/default/tests/WorkerThreadTest.cpp | 23 ++++---- 4 files changed, 78 insertions(+), 18 deletions(-) create mode 100644 biometrics/fingerprint/aidl/default/include/Callable.h diff --git a/biometrics/fingerprint/aidl/default/WorkerThread.cpp b/biometrics/fingerprint/aidl/default/WorkerThread.cpp index 512efb8d5d..d1a63d07ee 100644 --- a/biometrics/fingerprint/aidl/default/WorkerThread.cpp +++ b/biometrics/fingerprint/aidl/default/WorkerThread.cpp @@ -36,7 +36,7 @@ WorkerThread::~WorkerThread() { mThread.join(); } -bool WorkerThread::schedule(Task&& task) { +bool WorkerThread::schedule(std::unique_ptr task) { if (mIsDestructing) { return false; } @@ -58,10 +58,10 @@ void WorkerThread::threadFunc() { if (mIsDestructing) { return; } - Task task = std::move(mQueue.front()); + std::unique_ptr task = std::move(mQueue.front()); mQueue.pop_front(); lock.unlock(); - task(); + (*task)(); } } diff --git a/biometrics/fingerprint/aidl/default/include/Callable.h b/biometrics/fingerprint/aidl/default/include/Callable.h new file mode 100644 index 0000000000..c6295117e5 --- /dev/null +++ b/biometrics/fingerprint/aidl/default/include/Callable.h @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +namespace aidl::android::hardware::biometrics::fingerprint { + +// Interface for representing parameterless functions. Unlike std::function, this can also +// represent move-only lambdas. +class Callable { + public: + virtual void operator()() = 0; + virtual ~Callable() = default; + + // Creates a heap-allocated Callable instance from any function object. + template + static std::unique_ptr from(T func); + + private: + template + class AnyFuncWrapper; +}; + +// Private helper class for wrapping any function object into a Callable. +template +class Callable::AnyFuncWrapper : public Callable { + public: + explicit AnyFuncWrapper(T func) : mFunc(std::move(func)) {} + + void operator()() override { mFunc(); } + + private: + T mFunc; +}; + +template +std::unique_ptr Callable::from(T func) { + return std::make_unique>(std::move(func)); +} + +} // namespace aidl::android::hardware::biometrics::fingerprint \ No newline at end of file diff --git a/biometrics/fingerprint/aidl/default/include/WorkerThread.h b/biometrics/fingerprint/aidl/default/include/WorkerThread.h index 49104c85e2..6fff4f2cb7 100644 --- a/biometrics/fingerprint/aidl/default/include/WorkerThread.h +++ b/biometrics/fingerprint/aidl/default/include/WorkerThread.h @@ -21,9 +21,9 @@ #include #include -namespace aidl::android::hardware::biometrics::fingerprint { +#include "Callable.h" -using Task = std::function; +namespace aidl::android::hardware::biometrics::fingerprint { // A class that encapsulates a worker thread and a task queue, and provides a convenient interface // for a Session to schedule its tasks for asynchronous execution. @@ -47,7 +47,12 @@ class WorkerThread final { // If the internal queue is not full, pushes a task at the end of the queue and returns true. // Otherwise, returns false. If the queue is busy, blocks until it becomes available. - bool schedule(Task&& task); + // This method expects heap-allocated tasks because it's the simplest way to represent function + // objects of any type. Stack-allocated std::function could be used instead, but it cannot + // represent functions with move-only captures because std::function is inherently copyable. + // Not being able to pass move-only lambdas is a major limitation for the HAL implementation, + // so heap-allocated tasks that share a common interface (Callable) were chosen instead. + bool schedule(std::unique_ptr task); private: // The function that runs on the internal thread. Sequentially runs the available tasks from @@ -63,7 +68,7 @@ class WorkerThread final { std::atomic mIsDestructing; // Queue that's guarded by mQueueMutex and mQueueCond. - std::deque mQueue; + std::deque> mQueue; std::mutex mQueueMutex; std::condition_variable mQueueCond; diff --git a/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp b/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp index ba901ad5ca..0d5014bb84 100644 --- a/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp +++ b/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp @@ -25,13 +25,14 @@ namespace { +using aidl::android::hardware::biometrics::fingerprint::Callable; using aidl::android::hardware::biometrics::fingerprint::WorkerThread; using namespace std::chrono_literals; TEST(WorkerThreadTest, ScheduleReturnsTrueWhenQueueHasSpace) { WorkerThread worker(1 /*maxQueueSize*/); for (int i = 0; i < 100; ++i) { - EXPECT_TRUE(worker.schedule([] {})); + EXPECT_TRUE(worker.schedule(Callable::from([] {}))); // Allow enough time for the previous task to be processed. std::this_thread::sleep_for(2ms); } @@ -40,16 +41,16 @@ TEST(WorkerThreadTest, ScheduleReturnsTrueWhenQueueHasSpace) { TEST(WorkerThreadTest, ScheduleReturnsFalseWhenQueueIsFull) { WorkerThread worker(2 /*maxQueueSize*/); // Add a long-running task. - worker.schedule([] { std::this_thread::sleep_for(1s); }); + worker.schedule(Callable::from([] { std::this_thread::sleep_for(1s); })); // Allow enough time for the worker to start working on the previous task. std::this_thread::sleep_for(2ms); // Fill the worker's queue to the maximum. - worker.schedule([] {}); - worker.schedule([] {}); + worker.schedule(Callable::from([] {})); + worker.schedule(Callable::from([] {})); - EXPECT_FALSE(worker.schedule([] {})); + EXPECT_FALSE(worker.schedule(Callable::from([] {}))); } TEST(WorkerThreadTest, TasksExecuteInOrder) { @@ -58,19 +59,19 @@ TEST(WorkerThreadTest, TasksExecuteInOrder) { std::vector results; for (int i = 0; i < NUM_TASKS; ++i) { - worker.schedule([&results, i] { + worker.schedule(Callable::from([&results, i] { // Delay tasks differently to provoke races. std::this_thread::sleep_for(std::chrono::nanoseconds(100 - i % 100)); // Unguarded write to results to provoke races. results.push_back(i); - }); + })); } std::promise promise; auto future = promise.get_future(); // Schedule a special task to signal when all of the tasks are finished. - worker.schedule([&promise] { promise.set_value(); }); + worker.schedule(Callable::from([&promise] { promise.set_value(); })); auto status = future.wait_for(1s); ASSERT_EQ(status, std::future_status::ready); @@ -86,11 +87,11 @@ TEST(WorkerThreadTest, ExecutionStopsAfterWorkerIsDestroyed) { { WorkerThread worker(2 /*maxQueueSize*/); - worker.schedule([&promise1] { + worker.schedule(Callable::from([&promise1] { promise1.set_value(); std::this_thread::sleep_for(200ms); - }); - worker.schedule([&promise2] { promise2.set_value(); }); + })); + worker.schedule(Callable::from([&promise2] { promise2.set_value(); })); // Make sure the first task is executing. auto status1 = future1.wait_for(1s);