From 93e352b729193e2fd2d5e932e4aeb7116507ce43 Mon Sep 17 00:00:00 2001 From: Xinyu Li Date: Thu, 7 Mar 2024 14:25:28 -0800 Subject: [PATCH 1/5] fix error on dispose --- src/builder.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/builder.cc b/src/builder.cc index 27f342ce..c00cf4fd 100644 --- a/src/builder.cc +++ b/src/builder.cc @@ -72,6 +72,7 @@ Builder::Builder() Builder::~Builder() { + impl_->graphs.clear(); } Builder::Impl::~Impl() From aad95644368acf849c42e195ce61d2f2caafe3c1 Mon Sep 17 00:00:00 2001 From: Xinyu Li Date: Fri, 8 Mar 2024 11:18:26 -0800 Subject: [PATCH 2/5] use weak pointer --- include/ion/builder.h | 2 +- include/ion/graph.h | 9 ++++++--- src/builder.cc | 4 ++-- src/c_ion.cc | 3 ++- src/graph.cc | 26 ++++++++++++++++++-------- 5 files changed, 29 insertions(+), 15 deletions(-) diff --git a/include/ion/builder.h b/include/ion/builder.h index 5f0f97cf..c303f8a9 100644 --- a/include/ion/builder.h +++ b/include/ion/builder.h @@ -23,7 +23,7 @@ class DynamicModule; /** * Builder class is used to build graph, compile, run, save and load it. */ -class Builder { +class Builder : std::enable_shared_from_this { public: struct Impl; diff --git a/include/ion/graph.h b/include/ion/graph.h index ef7626fd..04f35e09 100644 --- a/include/ion/graph.h +++ b/include/ion/graph.h @@ -10,14 +10,16 @@ namespace ion { class Builder; class Graph { +public: - struct Impl; -public: + struct Impl; Graph(); - Graph(Builder builder, const std::string& name = ""); + + + Graph(std::shared_ptr builder , const std::string& name = ""); Graph& operator+=(const Graph& rhs); @@ -46,6 +48,7 @@ class Graph { private: std::shared_ptr impl_; + }; } // namespace ion diff --git a/src/builder.cc b/src/builder.cc index c00cf4fd..52c62102 100644 --- a/src/builder.cc +++ b/src/builder.cc @@ -72,11 +72,11 @@ Builder::Builder() Builder::~Builder() { - impl_->graphs.clear(); } Builder::Impl::~Impl() { + std::cout<<"Builder pointer is relaesed"<graphs.push_back(g); return g; } diff --git a/src/c_ion.cc b/src/c_ion.cc index 35798690..b04e05b1 100644 --- a/src/c_ion.cc +++ b/src/c_ion.cc @@ -1047,7 +1047,8 @@ int ion_port_map_set_buffer_array(ion_port_map_t obj, ion_port_t p, ion_buffer_t int ion_graph_create(ion_graph_t *ptr, ion_builder_t obj, const char * name) { try { - *ptr = reinterpret_cast(new Graph(*reinterpret_cast(obj), name)); + std::shared_ptr other_ptr(reinterpret_cast(obj)); + *ptr = reinterpret_cast(new Graph(other_ptr, name)); } catch (const Halide::Error& e) { log::error(e.what()); return 1; diff --git a/src/graph.cc b/src/graph.cc index b3ddff04..24a2c245 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -7,7 +7,7 @@ namespace ion { struct Graph::Impl { - Builder builder; + std::weak_ptr builder; std::string name; GraphID id; std::vector nodes; @@ -20,17 +20,25 @@ struct Graph::Impl { Impl() : id(sole::uuid4().str()) {} - Impl(Builder b, const std::string& n) + + Impl(std::shared_ptr b, const std::string& n) : id(sole::uuid4().str()), builder(b), name(n), jit_ctx(new Halide::JITUserContext), jit_ctx_ptr(jit_ctx.get()) { } + ~Impl(); }; Graph::Graph() { } -Graph::Graph(Builder builder, const std::string& name) + +Graph::Impl::~Impl() +{ + std::cout<<"Graph pointer is relaesed"< builder, const std::string& name) : impl_(new Impl(builder, name)) { } @@ -43,7 +51,7 @@ Graph& Graph::operator+=(const Graph& rhs) Graph operator+(const Graph& lhs, const Graph& rhs) { - Graph g(lhs.impl_->builder); + Graph g(lhs.impl_->builder.lock()); g += lhs; g += rhs; return g; @@ -51,7 +59,8 @@ Graph operator+(const Graph& lhs, const Graph& rhs) Node Graph::add(const std::string& name) { - auto n = impl_->builder.add(name,impl_->id); + auto ptr =impl_->builder.lock(); + auto n = ptr->add(name,impl_->id); impl_->nodes.push_back(n); return n; @@ -60,7 +69,7 @@ Node Graph::add(const std::string& name) void Graph::run() { if (!impl_->pipeline.defined()) { - impl_->pipeline = lower(impl_->builder, impl_->nodes, false); + impl_->pipeline = lower(*impl_->builder.lock(), impl_->nodes, false); if (!impl_->pipeline.defined()) { log::warn("This pipeline doesn't produce any outputs. Please bind a buffer with output port."); return; @@ -69,11 +78,11 @@ void Graph::run() if (!impl_->callable.defined()) { - impl_->pipeline.set_jit_externs(impl_->builder.jit_externs()); + impl_->pipeline.set_jit_externs(impl_->builder.lock()->jit_externs()); auto inferred_args = impl_->pipeline.infer_arguments(); - impl_->callable = impl_->pipeline.compile_to_callable(inferred_args, impl_->builder.target()); + impl_->callable = impl_->pipeline.compile_to_callable(inferred_args, impl_->builder.lock()->target()); impl_->args.clear(); impl_->args.push_back(&impl_->jit_ctx_ptr); @@ -94,4 +103,5 @@ std::vector& Graph::nodes() { } + } // namespace ion From 9a1d5fae1c3cb8f1725455d28ce0449dd74d42ad Mon Sep 17 00:00:00 2001 From: Xinyu Li Date: Fri, 8 Mar 2024 11:37:42 -0800 Subject: [PATCH 3/5] use reference --- include/ion/builder.h | 2 +- include/ion/graph.h | 2 +- src/builder.cc | 3 +-- src/c_ion.cc | 3 +-- src/graph.cc | 28 +++++++++------------------- 5 files changed, 13 insertions(+), 25 deletions(-) diff --git a/include/ion/builder.h b/include/ion/builder.h index c303f8a9..eddff040 100644 --- a/include/ion/builder.h +++ b/include/ion/builder.h @@ -23,7 +23,7 @@ class DynamicModule; /** * Builder class is used to build graph, compile, run, save and load it. */ -class Builder : std::enable_shared_from_this { +class Builder { public: struct Impl; diff --git a/include/ion/graph.h b/include/ion/graph.h index 04f35e09..f9bb75d7 100644 --- a/include/ion/graph.h +++ b/include/ion/graph.h @@ -19,7 +19,7 @@ class Graph { - Graph(std::shared_ptr builder , const std::string& name = ""); + Graph(Builder & builder , const std::string& name = ""); Graph& operator+=(const Graph& rhs); diff --git a/src/builder.cc b/src/builder.cc index 52c62102..27f342ce 100644 --- a/src/builder.cc +++ b/src/builder.cc @@ -76,7 +76,6 @@ Builder::~Builder() Builder::Impl::~Impl() { - std::cout<<"Builder pointer is relaesed"<graphs.push_back(g); return g; } diff --git a/src/c_ion.cc b/src/c_ion.cc index b04e05b1..35798690 100644 --- a/src/c_ion.cc +++ b/src/c_ion.cc @@ -1047,8 +1047,7 @@ int ion_port_map_set_buffer_array(ion_port_map_t obj, ion_port_t p, ion_buffer_t int ion_graph_create(ion_graph_t *ptr, ion_builder_t obj, const char * name) { try { - std::shared_ptr other_ptr(reinterpret_cast(obj)); - *ptr = reinterpret_cast(new Graph(other_ptr, name)); + *ptr = reinterpret_cast(new Graph(*reinterpret_cast(obj), name)); } catch (const Halide::Error& e) { log::error(e.what()); return 1; diff --git a/src/graph.cc b/src/graph.cc index 24a2c245..9cc790c8 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -7,7 +7,7 @@ namespace ion { struct Graph::Impl { - std::weak_ptr builder; + Builder & builder; std::string name; GraphID id; std::vector nodes; @@ -17,28 +17,18 @@ struct Graph::Impl { std::unique_ptr jit_ctx; Halide::JITUserContext* jit_ctx_ptr; std::vector args; - Impl() - : id(sole::uuid4().str()) - {} - Impl(std::shared_ptr b, const std::string& n) + Impl(Builder & b, const std::string& n) : id(sole::uuid4().str()), builder(b), name(n), jit_ctx(new Halide::JITUserContext), jit_ctx_ptr(jit_ctx.get()) { } - ~Impl(); }; Graph::Graph() { } - -Graph::Impl::~Impl() -{ - std::cout<<"Graph pointer is relaesed"< builder, const std::string& name) +Graph::Graph(Builder & builder, const std::string& name) : impl_(new Impl(builder, name)) { } @@ -51,7 +41,7 @@ Graph& Graph::operator+=(const Graph& rhs) Graph operator+(const Graph& lhs, const Graph& rhs) { - Graph g(lhs.impl_->builder.lock()); + Graph g(lhs.impl_->builder); g += lhs; g += rhs; return g; @@ -59,8 +49,8 @@ Graph operator+(const Graph& lhs, const Graph& rhs) Node Graph::add(const std::string& name) { - auto ptr =impl_->builder.lock(); - auto n = ptr->add(name,impl_->id); + auto ptr =impl_->builder; + auto n = ptr.add(name,impl_->id); impl_->nodes.push_back(n); return n; @@ -69,7 +59,7 @@ Node Graph::add(const std::string& name) void Graph::run() { if (!impl_->pipeline.defined()) { - impl_->pipeline = lower(*impl_->builder.lock(), impl_->nodes, false); + impl_->pipeline = lower(impl_->builder, impl_->nodes, false); if (!impl_->pipeline.defined()) { log::warn("This pipeline doesn't produce any outputs. Please bind a buffer with output port."); return; @@ -78,11 +68,11 @@ void Graph::run() if (!impl_->callable.defined()) { - impl_->pipeline.set_jit_externs(impl_->builder.lock()->jit_externs()); + impl_->pipeline.set_jit_externs(impl_->builder.jit_externs()); auto inferred_args = impl_->pipeline.infer_arguments(); - impl_->callable = impl_->pipeline.compile_to_callable(inferred_args, impl_->builder.lock()->target()); + impl_->callable = impl_->pipeline.compile_to_callable(inferred_args, impl_->builder.target()); impl_->args.clear(); impl_->args.push_back(&impl_->jit_ctx_ptr); From 90e5381c34880dcf8008cecfbddd375b025d27ce Mon Sep 17 00:00:00 2001 From: Xinyu Li Date: Fri, 8 Mar 2024 11:41:39 -0800 Subject: [PATCH 4/5] use reference --- include/ion/graph.h | 4 ---- src/graph.cc | 5 +---- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/include/ion/graph.h b/include/ion/graph.h index f9bb75d7..35efda6c 100644 --- a/include/ion/graph.h +++ b/include/ion/graph.h @@ -12,13 +12,10 @@ class Builder; class Graph { public: - struct Impl; Graph(); - - Graph(Builder & builder , const std::string& name = ""); Graph& operator+=(const Graph& rhs); @@ -48,7 +45,6 @@ class Graph { private: std::shared_ptr impl_; - }; } // namespace ion diff --git a/src/graph.cc b/src/graph.cc index 9cc790c8..9218db81 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -49,9 +49,7 @@ Graph operator+(const Graph& lhs, const Graph& rhs) Node Graph::add(const std::string& name) { - auto ptr =impl_->builder; - auto n = ptr.add(name,impl_->id); - + auto n = impl_->builder.add(name,impl_->id); impl_->nodes.push_back(n); return n; } @@ -93,5 +91,4 @@ std::vector& Graph::nodes() { } - } // namespace ion From b4e97e3bfe5464b4fff9bf2fe2ef714333d74e8d Mon Sep 17 00:00:00 2001 From: Xinyu Li Date: Fri, 8 Mar 2024 15:23:48 -0800 Subject: [PATCH 5/5] delete spave --- include/ion/builder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ion/builder.h b/include/ion/builder.h index eddff040..5f0f97cf 100644 --- a/include/ion/builder.h +++ b/include/ion/builder.h @@ -23,7 +23,7 @@ class DynamicModule; /** * Builder class is used to build graph, compile, run, save and load it. */ -class Builder { +class Builder { public: struct Impl;