From 6c93e42d94348d7912dcbe90a4b0a4d97a346768 Mon Sep 17 00:00:00 2001 From: Ruslan Garnov Date: Wed, 13 Nov 2019 14:58:58 +0300 Subject: [PATCH 1/2] Changed sharev_ptr for Priv to unique_ptr for fluid::Buffer --- .../gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp | 4 +++- modules/gapi/src/backends/fluid/gfluidbackend.cpp | 11 +++++------ modules/gapi/src/backends/fluid/gfluidbackend.hpp | 3 ++- modules/gapi/src/backends/fluid/gfluidbuffer.cpp | 3 +++ 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp b/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp index a5dd4a6f89..5c273c2041 100644 --- a/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp +++ b/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp @@ -115,6 +115,8 @@ public: BorderOpt border); // Constructor for in/out buffers (for tests) Buffer(const cv::gapi::own::Mat &data, bool is_input); + ~Buffer(); + Buffer& operator=(Buffer&&); inline uint8_t* OutLineB(int index = 0) { @@ -143,7 +145,7 @@ public: const Priv& priv() const; // internal use only private: - std::shared_ptr m_priv; + std::unique_ptr m_priv; const Cache* m_cache; }; diff --git a/modules/gapi/src/backends/fluid/gfluidbackend.cpp b/modules/gapi/src/backends/fluid/gfluidbackend.cpp index bdd4432c93..5fc9c4e211 100644 --- a/modules/gapi/src/backends/fluid/gfluidbackend.cpp +++ b/modules/gapi/src/backends/fluid/gfluidbackend.cpp @@ -833,7 +833,8 @@ cv::gimpl::GFluidExecutable::GFluidExecutable(const ade::Graph m_num_int_buffers (traverse_res.m_mat_count), m_scratch_users (traverse_res.m_scratch_users), m_id_map (traverse_res.m_id_map), - m_all_gmat_ids (traverse_res.m_all_gmat_ids) + m_all_gmat_ids (traverse_res.m_all_gmat_ids), + m_buffers(m_num_int_buffers + m_scratch_users.size()) { GConstFluidModel fg(m_g); @@ -857,9 +858,6 @@ cv::gimpl::GFluidExecutable::GFluidExecutable(const ade::Graph // Actually initialize Fluid buffers GAPI_LOG_INFO(NULL, "Initializing " << m_num_int_buffers << " fluid buffer(s)" << std::endl); - const std::size_t num_scratch = m_scratch_users.size(); - m_buffers.resize(m_num_int_buffers + num_scratch); - // After buffers are allocated, repack: ... for (auto &agent : m_agents) { @@ -905,6 +903,7 @@ cv::gimpl::GFluidExecutable::GFluidExecutable(const ade::Graph } // After parameters are there, initialize scratch buffers + const std::size_t num_scratch = m_scratch_users.size(); if (num_scratch) { GAPI_LOG_INFO(NULL, "Initializing " << num_scratch << " scratch buffer(s)" << std::endl); @@ -932,8 +931,8 @@ std::size_t cv::gimpl::GFluidExecutable::total_buffers_size() const for (const auto &i : ade::util::indexed(m_buffers)) { // Check that all internal and scratch buffers are allocated - const auto idx = ade::util::index(i); - const auto b = ade::util::value(i); + const auto idx = ade::util::index(i); + const auto& b = ade::util::value(i); if (idx >= m_num_int_buffers || fg.metadata(m_all_gmat_ids.at(idx)).get().internal == true) { diff --git a/modules/gapi/src/backends/fluid/gfluidbackend.hpp b/modules/gapi/src/backends/fluid/gfluidbackend.hpp index c8598d7da8..4aa9867dac 100644 --- a/modules/gapi/src/backends/fluid/gfluidbackend.hpp +++ b/modules/gapi/src/backends/fluid/gfluidbackend.hpp @@ -125,7 +125,6 @@ class GFluidExecutable final: public GIslandExecutable GModel::ConstGraph m_gm; std::vector> m_agents; - std::vector m_buffers; std::vector m_script; @@ -138,6 +137,8 @@ class GFluidExecutable final: public GIslandExecutable std::unordered_map m_id_map; // GMat id -> buffer idx map std::map m_all_gmat_ids; + std::vector m_buffers; + void bindInArg (const RcDesc &rc, const GRunArg &arg); void bindOutArg(const RcDesc &rc, const GRunArgP &arg); void packArg (GArg &in_arg, const GArg &op_arg); diff --git a/modules/gapi/src/backends/fluid/gfluidbuffer.cpp b/modules/gapi/src/backends/fluid/gfluidbuffer.cpp index e2a78078df..8ee2d98a14 100644 --- a/modules/gapi/src/backends/fluid/gfluidbuffer.cpp +++ b/modules/gapi/src/backends/fluid/gfluidbuffer.cpp @@ -680,6 +680,9 @@ fluid::Buffer::Buffer(const cv::gapi::own::Mat &data, bool is_input) m_priv->bindTo(data, is_input); } +fluid::Buffer::~Buffer() = default; +fluid::Buffer& fluid::Buffer::operator=(fluid::Buffer&&) = default; + int fluid::Buffer::linesReady() const { return m_priv->linesReady(); From 65ccafd494477310a036ccf1ec14b8f8f99d63c0 Mon Sep 17 00:00:00 2001 From: Ruslan Garnov Date: Wed, 13 Nov 2019 15:46:52 +0300 Subject: [PATCH 2/2] Changed shared_ptr to unique_ptr for Priv storage in fluid::View --- .../opencv2/gapi/fluid/gfluidbuffer.hpp | 8 ++++-- .../opencv2/gapi/fluid/gfluidkernel.hpp | 2 +- .../gapi/src/backends/fluid/gfluidbackend.cpp | 6 ++--- .../gapi/src/backends/fluid/gfluidbuffer.cpp | 25 ++++++++++++------- .../src/backends/fluid/gfluidbuffer_priv.hpp | 4 +-- modules/gapi/test/gapi_kernel_tests.cpp | 2 +- 6 files changed, 29 insertions(+), 18 deletions(-) diff --git a/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp b/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp index 5c273c2041..5c5f4bf6ee 100644 --- a/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp +++ b/modules/gapi/include/opencv2/gapi/fluid/gfluidbuffer.hpp @@ -85,10 +85,13 @@ public: Priv& priv(); // internal use only const Priv& priv() const; // internal use only - View(Priv* p); + View(std::unique_ptr&& p); + View(View&& v); + View& operator=(View&& v); + ~View(); private: - std::shared_ptr m_priv; + std::unique_ptr m_priv; const Cache* m_cache; }; @@ -139,6 +142,7 @@ public: inline const GMatDesc& meta() const { return m_cache->m_desc; } View mkView(int borderSize, bool ownStorage); + void addView(const View* v); class GAPI_EXPORTS Priv; // internal use only Priv& priv(); // internal use only diff --git a/modules/gapi/include/opencv2/gapi/fluid/gfluidkernel.hpp b/modules/gapi/include/opencv2/gapi/fluid/gfluidkernel.hpp index 3f7a0f811c..6f6ced4915 100644 --- a/modules/gapi/include/opencv2/gapi/fluid/gfluidkernel.hpp +++ b/modules/gapi/include/opencv2/gapi/fluid/gfluidkernel.hpp @@ -172,7 +172,7 @@ template<> struct fluid_get_in { static const cv::gapi::fluid::View& get(const cv::GArgs &in_args, int idx) { - return in_args[idx].unsafe_get(); + return *in_args[idx].unsafe_get(); } }; diff --git a/modules/gapi/src/backends/fluid/gfluidbackend.cpp b/modules/gapi/src/backends/fluid/gfluidbackend.cpp index 5fc9c4e211..9905f6f5a4 100644 --- a/modules/gapi/src/backends/fluid/gfluidbackend.cpp +++ b/modules/gapi/src/backends/fluid/gfluidbackend.cpp @@ -880,10 +880,10 @@ cv::gimpl::GFluidExecutable::GFluidExecutable(const ade::Graph auto inEdge = GModel::getInEdgeByPort(m_g, agent->op_handle, in_idx); auto ownStorage = fg.metadata(inEdge).get().use; - gapi::fluid::View view = buffer.mkView(fu.border_size, ownStorage); // NB: It is safe to keep ptr as view lifetime is buffer lifetime - agent->in_views[in_idx] = view; - agent->in_args[in_idx] = GArg(view); + agent->in_views[in_idx] = buffer.mkView(fu.border_size, ownStorage); + agent->in_args[in_idx] = GArg(&agent->in_views[in_idx]); + buffer.addView(&agent->in_views[in_idx]); } else { diff --git a/modules/gapi/src/backends/fluid/gfluidbuffer.cpp b/modules/gapi/src/backends/fluid/gfluidbuffer.cpp index 8ee2d98a14..1085a480a6 100644 --- a/modules/gapi/src/backends/fluid/gfluidbuffer.cpp +++ b/modules/gapi/src/backends/fluid/gfluidbuffer.cpp @@ -577,7 +577,7 @@ bool fluid::Buffer::Priv::full() const { // reset with maximum possible value and then find minimum slowest_y = m_desc.size.height; - for (const auto &v : m_views) slowest_y = std::min(slowest_y, v.y()); + for (const auto &v : m_views) slowest_y = std::min(slowest_y, v->y()); } return m_write_caret + lpi() - slowest_y > m_storage->rows(); @@ -609,7 +609,7 @@ void fluid::Buffer::Priv::reset() int fluid::Buffer::Priv::size() const { std::size_t view_sz = 0; - for (const auto &v : m_views) view_sz += v.priv().size(); + for (const auto &v : m_views) view_sz += v->priv().size(); auto total = view_sz; if (m_storage) total += m_storage->size(); @@ -693,17 +693,24 @@ int fluid::Buffer::lpi() const return m_priv->lpi(); } -fluid::View::View(Priv* p) - : m_priv(p), m_cache(&p->cache()) +fluid::View::View(std::unique_ptr&& p) + : m_priv(std::move(p)), m_cache(&m_priv->cache()) { /* nothing */ } +fluid::View::View(View&&) = default; +fluid::View& fluid::View::operator=(View&&) = default; +fluid::View::~View() = default; + fluid::View fluid::Buffer::mkView(int borderSize, bool ownStorage) { // FIXME: logic outside of Priv (because View takes pointer to Buffer) - auto view = ownStorage ? View(new ViewPrivWithOwnBorder(this, borderSize)) - : View(new ViewPrivWithoutOwnBorder(this, borderSize)); - m_priv->addView(view); - return view; + return ownStorage ? View(std::unique_ptr(new ViewPrivWithOwnBorder(this, borderSize))) + : View(std::unique_ptr(new ViewPrivWithoutOwnBorder(this, borderSize))); +} + +void fluid::Buffer::addView(const View* v) +{ + m_priv->addView(v); } void fluid::debugBufferPriv(const fluid::Buffer& buffer, std::ostream &os) @@ -717,7 +724,7 @@ void fluid::debugBufferPriv(const fluid::Buffer& buffer, std::ostream &os) <<" (phys " << "[" << p.storage().cols() << " x " << p.storage().rows() << "]" << ") :" << " w: " << p.m_write_caret << ", r: ["; - for (const auto &v : p.m_views) { os << &v.priv() << ":" << v.y() << " "; } + for (const auto &v : p.m_views) { os << &v->priv() << ":" << v->y() << " "; } os << "], avail: " << buffer.linesReady() << std::endl; } diff --git a/modules/gapi/src/backends/fluid/gfluidbuffer_priv.hpp b/modules/gapi/src/backends/fluid/gfluidbuffer_priv.hpp index 5925489ddf..5292b035ce 100644 --- a/modules/gapi/src/backends/fluid/gfluidbuffer_priv.hpp +++ b/modules/gapi/src/backends/fluid/gfluidbuffer_priv.hpp @@ -239,7 +239,7 @@ class GAPI_EXPORTS Buffer::Priv int m_write_caret = -1; - std::vector m_views; + std::vector m_views; std::unique_ptr m_storage; @@ -265,7 +265,7 @@ public: void allocate(BorderOpt border, int border_size, int line_consumption, int skew); void bindTo(const cv::gapi::own::Mat &data, bool is_input); - inline void addView(const View& view) { m_views.push_back(view); } + inline void addView(const View* view) { m_views.emplace_back(view); } inline const GMatDesc& meta() const { return m_desc; } diff --git a/modules/gapi/test/gapi_kernel_tests.cpp b/modules/gapi/test/gapi_kernel_tests.cpp index 6ed98ad4f4..13e5b21c7b 100644 --- a/modules/gapi/test/gapi_kernel_tests.cpp +++ b/modules/gapi/test/gapi_kernel_tests.cpp @@ -101,7 +101,7 @@ namespace GAPI_FLUID_KERNEL(GClone, I::GClone, false) { static const int Window = 1; - static void run(const cv::gapi::fluid::View&, cv::gapi::fluid::Buffer) + static void run(const cv::gapi::fluid::View&, cv::gapi::fluid::Buffer&) { HeteroGraph::registerCallKernel(KernelTags::FLUID_CUSTOM_CLONE); }