Merge pull request #14492 from anton-potapov:gapi_async_review_updates

modules/gapi/include/opencv2/gapi/gcompiled_async.hpp

@@ -8,7 +8,7 @@
 #ifndef OPENCV_GAPI_GCOMPILED_ASYNC_HPP
 #define OPENCV_GAPI_GCOMPILED_ASYNC_HPP
 
-#include <future>
+#include <future>           //for std::future
 #include <exception>        //for std::exception_ptr
 #include <functional>       //for std::function
 #include "opencv2/gapi/garg.hpp"
@@ -19,6 +19,9 @@ namespace cv {
 
 namespace gapi{
 namespace wip {
+    //These functions asynchronously (i.e. probably on a separate thread of execution) call operator() member function of their first argument with copies of rest of arguments (except callback) passed in.
+    //The difference between the function is the way to get the completion notification (via callback or a waiting on std::future object)
+    //If exception is occurred during execution of apply it is transfered to the callback (via function parameter) or passed to future (and will be thrown on call to std::future::get)
     GAPI_EXPORTS void                async(GCompiled& gcmpld, std::function<void(std::exception_ptr)>&& callback, GRunArgs &&ins, GRunArgsP &&outs);
     GAPI_EXPORTS std::future<void>   async(GCompiled& gcmpld, GRunArgs &&ins, GRunArgsP &&outs);
 } // namespace gapi

modules/gapi/include/opencv2/gapi/gcomputation_async.hpp

@@ -19,6 +19,9 @@ namespace cv {
     class GComputation;
 namespace gapi {
 namespace wip  {
+    //These functions asynchronously (i.e. probably on a separate thread of execution) call apply member function of their first argument with copies of rest of arguments (except callback) passed in.
+    //The difference between the function is the way to get the completion notification (via callback or a waiting on std::future object)
+    //If exception is occurred during execution of apply it is transfered to the callback (via function parameter) or passed to future (and will be thrown on call to std::future::get)
     GAPI_EXPORTS void                async_apply(GComputation& gcomp, std::function<void(std::exception_ptr)>&& callback, GRunArgs &&ins, GRunArgsP &&outs, GCompileArgs &&args = {});
     GAPI_EXPORTS std::future<void>   async_apply(GComputation& gcomp, GRunArgs &&ins, GRunArgsP &&outs, GCompileArgs &&args = {});
 } // nmaepspace gapi

modules/gapi/src/executor/gasync.cpp

@@ -13,7 +13,6 @@
 
 #include <future>
 #include <condition_variable>
-//#include <chrono>
 #include <stdexcept>
 #include <queue>
 
@@ -58,7 +57,7 @@ public:
             {
                 //have won (probable) race - so actually start the thread
                 thrd = std::thread {[this](){
-                    //move the whole queue into local instance in order to minimize time the protecting lock is held
+                    //move the whole queue into local instance in order to minimize time the guarding lock is held
                     decltype(q) second_q;
                     while (!exiting){
                         std::unique_lock<std::mutex> lck{mtx};
@@ -140,7 +139,7 @@ void call_with_futute(f_t&& f, std::promise<void>& p){
 }//namespace
 
 //For now these async functions are simply wrapping serial version of apply/operator() into a functor.
-//These functors are then serialized into single queue, which is when processed by a devoted background thread.
+//These functors are then serialized into single queue, which is processed by a devoted background thread.
 void async_apply(GComputation& gcomp, std::function<void(std::exception_ptr)>&& callback, GRunArgs &&ins, GRunArgsP &&outs, GCompileArgs &&args){
     //TODO: use move_through_copy for all args except gcomp
     auto l = [=]() mutable {

modules/gapi/test/gapi_async_test.cpp

@@ -14,20 +14,22 @@
 
 namespace opencv_test
 {
-struct SumOfSum{
+//Main idea behind these tests is to have the same test script that is parameterized in order to test all setups (GCompiled vs apply, callback vs future).
+//So these differences are factored into devoted helper classes (mixins) which are then used by the common test script by help of CRTP.
+//Actual GAPI Computation with parameters to run on is mixed into test via CRTP as well.
+
+struct SumOfSum2x2 {
     cv::GComputation sum_of_sum;
-    SumOfSum() : sum_of_sum([]{
+    SumOfSum2x2() : sum_of_sum([]{
         cv::GMat in;
         cv::GScalar out = cv::gapi::sum(in + in);
         return GComputation{in, out};
     })
     {}
-};
 
-struct SumOfSum2x2 : SumOfSum {
     const cv::Size sz{2, 2};
     cv::Mat in_mat{sz, CV_8U, cv::Scalar(1)};
-    cv::Scalar out;
+    cv::Scalar out_sc;
 
     cv::GCompiled compile(){
         return sum_of_sum.compile(descr_of(in_mat));
@@ -46,11 +48,11 @@ struct SumOfSum2x2 : SumOfSum {
     }
 
     cv::GRunArgsP out_args(){
-        return cv::gout(out);
+        return cv::gout(out_sc);
     }
 
     void verify(){
-        EXPECT_EQ(8, out[0]);
+        EXPECT_EQ(8, out_sc[0]);
     }
 };
 
@@ -78,7 +80,7 @@ namespace {
     };
 }
 
-struct ExceptionOnExecution  {
+struct ExceptionOnExecution {
     cv::GComputation throwing_gcomp;
     ExceptionOnExecution() : throwing_gcomp([]{
         cv::GMat in;
@@ -124,6 +126,7 @@ struct crtp_cast {
     }
 };
 
+//Test Mixin, hiding details of callback based notification
 template<typename crtp_final_t>
 struct CallBack: crtp_cast<crtp_final_t> {
     std::atomic<bool> callback_called = {false};
@@ -158,6 +161,7 @@ struct CallBack: crtp_cast<crtp_final_t> {
     }
 };
 
+//Test Mixin, hiding details of future based notification
 template<typename crtp_final_t>
 struct Future: crtp_cast<crtp_final_t> {
     std::future<void> f;
@@ -173,7 +177,7 @@ struct Future: crtp_cast<crtp_final_t> {
     }
 };
 
-
+//Test Mixin, hiding details of using compiled GAPI object
 template<typename crtp_final_t>
 struct AsyncCompiled  : crtp_cast<crtp_final_t>{
 
@@ -184,6 +188,7 @@ struct AsyncCompiled  : crtp_cast<crtp_final_t>{
     }
 };
 
+//Test Mixin, hiding details of calling apply (async_apply) on GAPI Computation object
 template<typename crtp_final_t>
 struct AsyncApply : crtp_cast<crtp_final_t> {
 
@@ -200,6 +205,7 @@ struct normal: ::testing::Test, case_t{};
 TYPED_TEST_CASE_P(normal);
 
 TYPED_TEST_P(normal, basic){
+    //Normal scenario:  start function asynchronously and wait for the result, and verify it
     this->start_async(this->in_args(), this->out_args());
     this->wait_for_result();
 
@@ -215,6 +221,7 @@ struct exception: ::testing::Test, case_t{};
 TYPED_TEST_CASE_P(exception);
 
 TYPED_TEST_P(exception, basic){
+    //Exceptional scenario:  start function asynchronously and make sure exception is passed to the user
     this->start_async(this->in_args(), this->out_args());
     EXPECT_THROW(this->wait_for_result(), gthrow_exception);
 }
@@ -228,6 +235,7 @@ struct stress : ::testing::Test{};
 TYPED_TEST_CASE_P(stress);
 
 TYPED_TEST_P(stress, test){
+    //Some stress testing: use a number of threads to start a bunch of async requests
     const std::size_t request_per_thread = 10;
     const std::size_t number_of_threads  = 4;
 
@@ -254,6 +262,7 @@ TYPED_TEST_P(stress, test){
 }
 REGISTER_TYPED_TEST_CASE_P(stress, test);
 
+//little helpers to match up all combinations of setups
 template<typename compute_fixture_t,template <typename> class callback_or_future_t, template <typename> class compiled_or_apply_t>
 struct Case
         : compute_fixture_t,