Have DumpPool return futures
Have DumpPool return futures when tasks are enqueued, instead of
having users register their task with a string. This change has the
following benefits:
* Simplifies DumpPool since it no longer needs to keep the map of
strings to futures.
* Allows adding a set of tasks to DumpPool that might not be known at
build time. For example, we can add a bunch of tasks from a vector,
without needing to generate arbitrary strings for them.
* Reduces the ability to misuse DumpPool. Before this change, the user
had to come up with strings representing each task, and needed to
know a task with said name existed. This change moves it from being a
runtime error to being a compile time one.
Test: atest dumpstate_smoke_test
Change-Id: Ic6c1a983b049e5236ac596e8af200a21c07ac31b
diff --git a/cmds/dumpstate/DumpPool.h b/cmds/dumpstate/DumpPool.h
index 0c3c2cc..9fb0fcc 100644
--- a/cmds/dumpstate/DumpPool.h
+++ b/cmds/dumpstate/DumpPool.h
@@ -18,7 +18,6 @@
#define FRAMEWORK_NATIVE_CMD_DUMPPOOL_H_
#include <future>
-#include <map>
#include <queue>
#include <string>
@@ -32,8 +31,26 @@
class DumpPoolTest;
/*
+ * Waits until the task is finished. Dumps the task results to the specified
+ * out_fd.
+ *
+ * |future| The task future.
+ * |title| Dump title string to the out_fd, an empty string for nothing.
+ * |out_fd| The target file to dump the result from the task.
+ */
+void WaitForTask(std::future<std::string> future, const std::string& title, int out_fd);
+
+/*
+ * Waits until the task is finished. Dumps the task results to the STDOUT_FILENO.
+ */
+
+inline void WaitForTask(std::future<std::string> future) {
+ WaitForTask(std::move(future), "", STDOUT_FILENO);
+}
+
+/*
* A thread pool with the fixed number of threads to execute multiple dump tasks
- * simultaneously for the dumpstate. The dump task is a callable function. It
+ * simultaneously for dumpstate. The dump task is a callable function. It
* could include a file descriptor as a parameter to redirect dump results, if
* it needs to output results to the bugreport. This can avoid messing up
* bugreport's results when multiple dump tasks are running at the same time.
@@ -44,13 +61,16 @@
* }
* ...
* DumpPool pool(tmp_root);
- * pool.enqueueTaskWithFd("TaskName", &DumpFoo, std::placeholders::_1);
+ * auto task = pool.enqueueTaskWithFd("TaskName", &DumpFoo, std::placeholders::_1);
* ...
- * pool.waitForTask("TaskName");
+ * WaitForTask(task);
*
* DumpFoo is a callable function included a out_fd parameter. Using the
* enqueueTaskWithFd method in DumpPool to enqueue the task to the pool. The
* std::placeholders::_1 is a placeholder for DumpPool to pass a fd argument.
+ *
+ * std::futures returned by `enqueueTask*()` must all have their `get` methods
+ * called, or have been destroyed before the DumpPool itself is destroyed.
*/
class DumpPool {
friend class android::os::dumpstate::DumpPoolTest;
@@ -63,6 +83,12 @@
* files.
*/
explicit DumpPool(const std::string& tmp_root);
+
+ /*
+ * Will waits until all threads exit the loop. Destroying DumpPool before destroying the
+ * associated std::futures created by `enqueueTask*` will cause an abort on Android because
+ * Android is built with `-fno-exceptions`.
+ */
~DumpPool();
/*
@@ -73,68 +99,47 @@
void start(int thread_counts = MAX_THREAD_COUNT);
/*
- * Requests to shutdown the pool and waits until all threads exit the loop.
- */
- void shutdown();
-
- /*
* Adds a task into the queue of the thread pool.
*
- * |task_name| The name of the task. It's also the title of the
+ * |duration_title| The name of the task. It's also the title of the
* DurationReporter log.
* |f| Callable function to execute the task.
* |args| A list of arguments.
*
* TODO(b/164369078): remove this api to have just one enqueueTask for consistency.
*/
- template<class F, class... Args> void enqueueTask(const std::string& task_name, F&& f,
- Args&&... args) {
+ template<class F, class... Args>
+ std::future<std::string> enqueueTask(const std::string& duration_title, F&& f, Args&&... args) {
std::function<void(void)> func = std::bind(std::forward<F>(f),
std::forward<Args>(args)...);
- futures_map_[task_name] = post(task_name, func);
+ auto future = post(duration_title, func);
if (threads_.empty()) {
start();
}
+ return future;
}
/*
* Adds a task into the queue of the thread pool. The task takes a file
* descriptor as a parameter to redirect dump results to a temporary file.
*
- * |task_name| The name of the task. It's also the title of the
- * DurationReporter log.
+ * |duration_title| The title of the DurationReporter log.
* |f| Callable function to execute the task.
* |args| A list of arguments. A placeholder std::placeholders::_1 as a fd
* argument needs to be included here.
*/
- template<class F, class... Args> void enqueueTaskWithFd(const std::string& task_name, F&& f,
- Args&&... args) {
+ template<class F, class... Args> std::future<std::string> enqueueTaskWithFd(
+ const std::string& duration_title, F&& f, Args&&... args) {
std::function<void(int)> func = std::bind(std::forward<F>(f),
std::forward<Args>(args)...);
- futures_map_[task_name] = post(task_name, func);
+ auto future = post(duration_title, func);
if (threads_.empty()) {
start();
}
+ return future;
}
/*
- * Waits until the task is finished. Dumps the task results to the STDOUT_FILENO.
- */
- void waitForTask(const std::string& task_name) {
- waitForTask(task_name, "", STDOUT_FILENO);
- }
-
- /*
- * Waits until the task is finished. Dumps the task results to the specified
- * out_fd.
- *
- * |task_name| The name of the task.
- * |title| Dump title string to the out_fd, an empty string for nothing.
- * |out_fd| The target file to dump the result from the task.
- */
- void waitForTask(const std::string& task_name, const std::string& title, int out_fd);
-
- /*
* Deletes temporary files created by DumpPool.
*/
void deleteTempFiles();
@@ -143,22 +148,22 @@
private:
using Task = std::packaged_task<std::string()>;
- using Future = std::shared_future<std::string>;
template<class T> void invokeTask(T dump_func, const std::string& duration_title, int out_fd);
- template<class T> Future post(const std::string& task_name, T dump_func) {
+ template<class T>
+ std::future<std::string> post(const std::string& duration_title, T dump_func) {
Task packaged_task([=]() {
std::unique_ptr<TmpFile> tmp_file_ptr = createTempFile();
if (!tmp_file_ptr) {
return std::string("");
}
- invokeTask(dump_func, task_name, tmp_file_ptr->fd.get());
+ invokeTask(dump_func, duration_title, tmp_file_ptr->fd.get());
fsync(tmp_file_ptr->fd.get());
return std::string(tmp_file_ptr->path);
});
std::unique_lock lock(lock_);
- auto future = packaged_task.get_future().share();
+ auto future = packaged_task.get_future();
tasks_.push(std::move(packaged_task));
condition_variable_.notify_one();
return future;
@@ -194,7 +199,6 @@
std::vector<std::thread> threads_;
std::queue<Task> tasks_;
- std::map<std::string, Future> futures_map_;
DISALLOW_COPY_AND_ASSIGN(DumpPool);
};