From 1128b46e5e287dc9d339396373a5a427ac254b48 Mon Sep 17 00:00:00 2001 From: arunmu Date: Wed, 16 Mar 2016 22:01:36 +0530 Subject: [PATCH] Perf improvement --- subprocess.hpp | 91 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 29 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 1c00d0f..7dc08f9 100755 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -1,14 +1,15 @@ #include +#include #include #include #include #include #include #include +#include #include #include #include -#include #include #include @@ -141,18 +142,18 @@ namespace util } static - int wait_for_child_exit(int pid) + std::pair wait_for_child_exit(int pid) { - int status; + int status = 0; int ret = -1; while (1) { ret = waitpid(pid, &status, WNOHANG); if (ret == -1) break; if (ret == 0) continue; - return pid; + return std::make_pair(ret, status); } - return ret; + return std::make_pair(ret, status); } @@ -276,6 +277,7 @@ public: Buffer() {} Buffer(size_t cap) { buf.resize(cap); } void add_cap(size_t cap) { buf.resize(cap); } + public: std::vector buf; size_t length = 0; @@ -474,6 +476,8 @@ public: int pid() const noexcept { return child_pid_; } + bool wait() throw(OSError); + void set_out_buf_cap(size_t cap) { stream_.set_out_buf_cap(cap); } void set_err_buf_cap(size_t cap) { stream_.set_err_buf_cap(cap); } @@ -485,10 +489,18 @@ public: { return stream_.send(msg); } std::pair communicate(const char* msg, size_t length) - { return stream_.communicate(msg, length); } + { + auto res = stream_.communicate(msg, length); + wait(); + return res; + } std::pair communicate(const std::vector& msg) - { return stream_.communicate(msg); } + { + auto res = stream_.communicate(msg); + wait(); + return res; + } private: template @@ -548,6 +560,19 @@ void Popen::start_process() throw (CalledProcessError, OSError) execute_process(); } +bool Popen::wait() throw (OSError) +{ + int ret, status; + std::tie(ret, status) = util::wait_for_child_exit(pid()); + if (ret == -1) { + if (errno != ECHILD) throw OSError("waitpid failed", errno); + return true; + } + if (!WIFEXITED(status)) return false; + + return true; +} + void Popen::execute_process() throw (CalledProcessError, OSError) { @@ -597,8 +622,7 @@ void Popen::execute_process() throw (CalledProcessError, OSError) // Call waitpid to reap the child process // waitpid suspends the calling process until the // child terminates. - sys_ret = util::wait_for_child_exit(child_pid_); - if (sys_ret == -1) throw OSError("child exit", errno); + wait(); // Throw whatever information we have about child failure throw CalledProcessError(err_buf); @@ -830,7 +854,7 @@ namespace detail { // Close the error stream stream_->error_.reset(); } - return std::make_pair(obuf, ebuf); + return std::make_pair(std::move(obuf), std::move(ebuf)); } return communicate_threaded(msg, length); @@ -843,36 +867,31 @@ namespace detail { OutBuffer obuf; ErrBuffer ebuf; - std::vector bg_threads; + std::future out_fut, err_fut; if (stream_->output()) { obuf.add_cap(out_buf_cap_); - bg_threads.push_back( - std::thread( - util::read_atmost_n, - fileno(stream_->output()), - obuf.buf.data(), - obuf.buf.size()) - ); + out_fut = std::async(std::launch::async, + util::read_atmost_n, + fileno(stream_->output()), + obuf.buf.data(), + obuf.buf.size()); } if (stream_->error()) { ebuf.add_cap(err_buf_cap_); - bg_threads.push_back( - std::thread( - util::read_atmost_n, - fileno(stream_->error()), - ebuf.buf.data(), - ebuf.buf.size()) - ); + err_fut = std::async(std::launch::async, + util::read_atmost_n, + fileno(stream_->error()), + ebuf.buf.data(), + ebuf.buf.size()); } if (stream_->input()) { if (msg) { int wbytes = std::fwrite(msg, sizeof(char), length, stream_->input()); if (wbytes < length) { if (errno != EPIPE && errno != EINVAL) { - for (auto& t: bg_threads) t.join(); throw OSError("fwrite error", errno); } } @@ -880,12 +899,26 @@ namespace detail { stream_->input_.reset(); } - // Wait for the threads to finish - for(auto& thr : bg_threads) thr.join(); + if (out_fut.valid()) { + int res = out_fut.get(); + if (res != -1) obuf.length = res; + else obuf.length = 0; + } + if (err_fut.valid()) { + int res = err_fut.get(); + if (res != -1) ebuf.length = res; + else ebuf.length = 0; + } - return std::make_pair(obuf, ebuf); + return std::make_pair(std::move(obuf), std::move(ebuf)); } }; // end namespace detail + + +// Convenience Functions +// + + };