Repository: fixstars/ion-kit (language: python)

Issue:
Eliminate json.hpp dependency from public header



Discussion / hints:

Fix patch:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 69ca04d6..899952dd 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -60,7 +60,6 @@ endif()
 # Build main library
 #
 file(GLOB ION_CORE_SRC LIST_DIRECTORIES false ${PROJECT_SOURCE_DIR}/src/*)
-list(REMOVE_ITEM ION_CORE_SRC "${PROJECT_SOURCE_DIR}/src/generator.cc")
 add_library(ion-core SHARED ${ION_CORE_SRC})
 target_include_directories(ion-core PUBLIC ${PROJECT_SOURCE_DIR}/include ${PROJECT_SOURCE_DIR}/src)
 if (UNIX)
diff --git a/README.md b/README.md
index 79260739..f3100e5f 100644
--- a/README.md
+++ b/README.md
@@ -3,8 +3,9 @@
 [![Windows](https://github.com/fixstars/ion-kit/workflows/Windows/badge.svg)](https://github.com/fixstars/ion-kit/actions?query=workflow%3AWindows)
 
 # ion-kit
-ion-kit is a graph based image processing framework based on Halide.
-User can build pipeline using ion-kit API composing building blocks and compile it into static library or run just in time.
+The ion-kit is a graph based data processing framework.
+User can define data processing algorithm in Halide as a "Building Block" (BB), then form a processing pipeline as a directed acyclic graph (DAG) combining BBs.
+The pipeline can be ran just in time or compiled into binary object.
 
 ## Depedencies
 * [Halide (v16.0.0)](https://github.com/halide/Halide/releases/tag/v16.0.0)
@@ -12,38 +13,78 @@ User can build pipeline using ion-kit API composing building blocks and compile
 ## Quick start
 
 ```c++
+#include <ion/ion.h>
+
+struct MyFilter : ion::BuildingBlock<MyFilter> {
+    // This Building Block takes 1 input, 1 output and 1 parameter.
+    ion::Input<Halide::Func> input{"input", Int(32), 1};
+    ion::Output<Halide::Func> output{"output", Int(32), 1};
+    ion::GeneratorParam<int32_t> v{"v", 0};
+
+    void generate() {
+        Halide::Var i;
+
+        // Increment input elements by value specified as "v"
+        output(i) = input(i) + v;
+    }
+};
+ION_REGISTER_BUILDING_BLOCK(MyFilter, my_filter);
+
 int main() {
-    int32_t min0 = 0, extent0 = 2, min1 = 0, extent1 = 2, v = 1;
+    int32_t v = 1;
 
-    // ion::Builder is fundamental class to build a graph.
+    // ion::Builder is the fundamental class to build a graph.
     ion::Builder b;
 
-    // Load ion building block module. User can create own BB module using ion::BuildingBlock class.
-    b.with_bb_module("ion-bb-test");
-
-    // Set the target architecture you will compile to. Here just use host architecture.
+    // Set the target architecture same as host.
     b.set_target(ion::get_host_target());
 
-    // Create simple graph consists from two nodes.
+    auto size = 4;
+
+    ion::Buffer<int32_t> input{size};
+    input.fill(0);
+
+    // Create sequential graph.
     //
-    // test_producer -> test_consumer -> r (dummy output)
+    // input -> my_filter (1st) -> my_filter (2nd) -> output
     //
-    ion::Node n;
-    n = b.add("test_producer").set_param(ion::Param("v", 41));
-    n = b.add("test_consumer")(n["output"], &min0, &extent0, &min1, &extent1, &v);
 
-    // Allocate dummy output. At least one output is required to run the pipeline.
-    auto r = ion::Buffer<int32_t>::make_scalar();
+    // Builder::add() creates Node object from Building Block.
+    ion::Node n1 = b.add("my_filter");
 
-    // Bind output with test_consumer "output" port.
-    n["output"].bind(r);
+    // Input is set by calling Node::operator().
+    n1(input);
 
-    // Run the pipeline. Internally, it is compiled into native code just in time called as a function.
+    // Parameter can be set by Node::set_param();
+    n1.set_param(ion::Param("v", 40));
+
+    // Method chain can be used to make it simple.
+    auto n2 = b.add("my_filter")(n1["output"]).set_param(ion::Param("v", 2));
+
+    // Bind output buffer.
+    ion::Buffer<int32_t> output{size};
+    output.fill(0);
+    n2["output"].bind(output);
+
+    // Run the pipeline.
     b.run();
+
+    // Expected output is "42 42 42 42"
+    for (int i=0; i<size; ++i) {
+        std::cout << output(i) << " ";
+    }
+    std::cout << std::endl;
+
+    return 0;
 }
 ```
 
-Compile it.
+Assuming [release binary](https://github.com/fixstars/ion-kit/releases) is extracted in <ion-kit-PATH>.
+
+```bash
+$ c++ -std=c++17 -fno-rtti main.cc -o main -I <ion-kit-PATH>/include -L <ion-kit-PATH>/lib -lion-core -lHalide && LD_LIBRARY_PATH=<ion-kit-PATH>/lib ./main
+42 42 42 42
+```
 
 ## Build from scratch
 Please follow the instructions provided for your preferred platform.
diff --git a/include/ion/builder.h b/include/ion/builder.h
index 3641e83c..527de95e 100644
--- a/include/ion/builder.h
+++ b/include/ion/builder.h
@@ -1,7 +1,6 @@
 #ifndef ION_BUILDER_H
 #define ION_BUILDER_H
 
-#include <deque>
 #include <string>
 #include <vector>
 #include <unordered_map>
diff --git a/include/ion/building_block.h b/include/ion/building_block.h
index 3bc2dfee..2c621507 100644
--- a/include/ion/building_block.h
+++ b/include/ion/building_block.h
@@ -2,6 +2,7 @@
 #define ION_BUILDING_BLOCK_H
 
 #include <vector>
+
 #include <Halide.h>
 
 #include "builder.h"
diff --git a/include/ion/node.h b/include/ion/node.h
index 869b2dee..78eff26f 100644
--- a/include/ion/node.h
+++ b/include/ion/node.h
@@ -17,8 +17,8 @@ namespace ion {
  */
 class Node {
     friend class Builder;
-    friend struct nlohmann::adl_serializer<Node>;
 
+public:
     struct Impl {
         std::string id;
         std::string name;
@@ -35,6 +35,8 @@ class Node {
 public:
     Node() : impl_(new Impl) {};
 
+    Node(const std::shared_ptr<Impl>& impl) : impl_(impl) {};
+
     /**
      * Set the target of the node.
      * @arg target: The target ofject which consists of OS, Architecture, and sets of Features.
@@ -87,6 +89,7 @@ class Node {
      */
     Port operator[](const std::string& name);
 
+    // Getter
     const std::string& id() const {
         return impl_->id;
     }
@@ -103,6 +106,10 @@ class Node {
         return impl_->params;
     }
 
+    const std::vector<Port>& ports() const {
+        return impl_->ports;
+    }
+
    std::vector<Port> iports() const {
         std::vector<Port> iports;
         for (const auto& p: impl_->ports) {
diff --git a/include/ion/port.h b/include/ion/port.h
index cfd7365c..0525cfaf 100644
--- a/include/ion/port.h
+++ b/include/ion/port.h
@@ -11,8 +11,6 @@
 
 #include <Halide.h>
 
-#include "json/json.hpp"
-
 #include "util.h"
 
 namespace ion {
@@ -46,6 +44,9 @@ int32_t unify_dimension(const std::vector<Halide::Buffer<T>>& bufs) {
  * Port class is used to create dynamic i/o for each node.
  */
 class Port {
+    friend class Builder;
+    friend class Node;
+
 public:
     using Channel = std::tuple<std::string, std::string>;
 
@@ -75,75 +76,70 @@ class Port {
         }
     };
 
- public:
-     friend class Builder;
-     friend class Node;
-     friend struct nlohmann::adl_serializer<Port>;
-
-     Port() : impl_(new Impl("", "", Halide::Type(), 0)), index_(-1) {}
-
-     Port(const std::shared_ptr<Impl>& impl) : impl_(impl), index_(-1) {}
-
-     /**
-      * Construct new port for scalar value.
-      * @arg k: The key of the port which should be matched with BuildingBlock Input/Output name.
-      * @arg t: The type of the value.
-      */
-     Port(const std::string& n, Halide::Type t) : impl_(new Impl("", n, t, 0)), index_(-1) {}
-
-     /**
-      * Construct new port for vector value.
-      * @arg k: The key of the port which should be matched with BuildingBlock Input/Output name.
-      * @arg t: The type of the element value.
-      * @arg d: The dimension of the port. The range is 1 to 4.
-      */
-     Port(const std::string& n, Halide::Type t, int32_t d) : impl_(new Impl("", n, t, d)), index_(-1) {}
-
-
-     /**
-      * Construct new port from scalar pointer
-      */
-     template<typename T,
-              typename std::enable_if<std::is_arithmetic<T>::value>::type* = nullptr>
-     Port(T *vptr) : impl_(new Impl("", Halide::Internal::unique_name("ion_port"), Halide::type_of<T>(), 0)), index_(-1) {
-         this->bind(vptr);
-     }
-
-     /**
-      * Construct new port from buffer
-      */
-     template<typename T>
-     Port(const Halide::Buffer<T>& buf) : impl_(new Impl("", buf.name(), Halide::type_of<T>(), buf.dimensions())), index_(-1) {
-         this->bind(buf);
-     }
-
-     /**
-      * Construct new port from array of buffer
-      */
-     template<typename T>
-     Port(const std::vector<Halide::Buffer<T>>& bufs) : impl_(new Impl("", unify_name(bufs), Halide::type_of<T>(), unify_dimension(bufs))), index_(-1) {
-         this->bind(bufs);
-     }
-
-     const std::string& pred_id() const { return std::get<0>(impl_->pred_chan); }
-     const std::string& pred_name() const { return std::get<1>(impl_->pred_chan); }
+public:
 
-     const Halide::Type& type() const { return impl_->type; }
+    Port() : impl_(new Impl("", "", Halide::Type(), 0)), index_(-1) {}
 
-     int32_t dimensions() const { return impl_->dimensions; }
+    Port(const std::shared_ptr<Impl>& impl, int32_t index) : impl_(impl), index_(index) {}
 
-     // const std::string& succ_id() const { return impl_->succ_id; }
+    /**
+     * Construct new port for scalar value.
+     * @arg k: The key of the port which should be matched with BuildingBlock Input/Output name.
+     * @arg t: The type of the value.
+     */
+    Port(const std::string& n, Halide::Type t) : impl_(new Impl("", n, t, 0)), index_(-1) {}
 
-     int32_t size() const { return static_cast<int32_t>(impl_->params.size()); }
+    /**
+     * Construct new port for vector value.
+     * @arg k: The key of the port which should be matched with BuildingBlock Input/Output name.
+     * @arg t: The type of the element value.
+     * @arg d: The dimension of the port. The range is 1 to 4.
+     */
+    Port(const std::string& n, Halide::Type t, int32_t d) : impl_(new Impl("", n, t, d)), index_(-1) {}
 
-     int32_t index() const { return index_; }
+    /**
+     * Construct new port from scalar pointer
+     */
+    template<typename T,
+             typename std::enable_if<std::is_arithmetic<T>::value>::type* = nullptr>
+    Port(T *vptr) : impl_(new Impl("", Halide::Internal::unique_name("ion_port"), Halide::type_of<T>(), 0)), index_(-1) {
+        this->bind(vptr);
+    }
 
-     bool has_pred() const { return !std::get<0>(impl_->pred_chan).empty(); }
+    /**
+     * Construct new port from buffer
+     */
+    template<typename T>
+    Port(const Halide::Buffer<T>& buf) : impl_(new Impl("", buf.name(), Halide::type_of<T>(), buf.dimensions())), index_(-1) {
+        this->bind(buf);
+    }
 
-     bool has_succ() const { return !impl_->succ_chans.empty(); }
-     bool has_succ(const Channel& c) const { return impl_->succ_chans.count(c); }
+    /**
+     * Construct new port from array of buffer
+     */
+    template<typename T>
+    Port(const std::vector<Halide::Buffer<T>>& bufs) : impl_(new Impl("", unify_name(bufs), Halide::type_of<T>(), unify_dimension(bufs))), index_(-1) {
+        this->bind(bufs);
+    }
 
-     void set_index(int index) { index_ = index; }
+    // Getter
+    const Channel& pred_chan() const { return impl_->pred_chan; }
+    const std::string& pred_id() const { return std::get<0>(impl_->pred_chan); }
+    const std::string& pred_name() const { return std::get<1>(impl_->pred_chan); }
+    const std::set<Channel>& succ_chans() const { return impl_->succ_chans; }
+    const Halide::Type& type() const { return impl_->type; }
+    int32_t dimensions() const { return impl_->dimensions; }
+    int32_t size() const { return static_cast<int32_t>(impl_->params.size()); }
+    int32_t index() const { return index_; }
+    uintptr_t impl_ptr() const { return reinterpret_cast<uintptr_t>(impl_.get()); }
+
+    // Setter
+    void set_index(int index) { index_ = index; }
+
+    // Util
+    bool has_pred() const { return !std::get<0>(impl_->pred_chan).empty(); }
+    bool has_succ() const { return !impl_->succ_chans.empty(); }
+    bool has_succ(const Channel& c) const { return impl_->succ_chans.count(c); }
 
     /**
      * Overloaded operator to set the port index and return a reference to the current port. eg. port[0]
@@ -192,14 +188,16 @@ class Port {
          }
      }
 
-     static std::shared_ptr<Impl> find_impl(uintptr_t ptr) {
+     static std::tuple<std::shared_ptr<Impl>, bool> find_impl(uintptr_t ptr) {
          static std::unordered_map<uintptr_t, std::shared_ptr<Impl>> impls;
          static std::mutex mutex;
          std::scoped_lock lock(mutex);
+         bool found = true;
          if (!impls.count(ptr)) {
              impls[ptr] = std::make_shared<Impl>();
+             found = false;
          }
-         return impls[ptr];
+         return std::make_tuple(impls[ptr], found);
      }
 
 private:
diff --git a/src/builder.cc b/src/builder.cc
index 5d380391..33fd17e8 100644
--- a/src/builder.cc
+++ b/src/builder.cc
@@ -87,9 +87,7 @@ Builder::Builder()
 
 Builder::~Builder()
 {
-    for (auto kv : disposers_) {
-        auto bb_id(std::get<0>(kv));
-        auto disposer(std::get<1>(kv));
+    for (auto [bb_id, disposer] : disposers_) {
         disposer(bb_id.c_str());
     }
 }
diff --git a/src/serializer.h b/src/serializer.h
index 747967ab..8e0c8921 100644
--- a/src/serializer.h
+++ b/src/serializer.h
@@ -42,50 +42,55 @@ static void from_json(const json& j, ion::Param& v) {
 template<>
 struct adl_serializer<ion::Port> {
      static void to_json(json& j, const ion::Port& v) {
-         j["pred_chan"] = v.impl_->pred_chan;
-         j["succ_chans"] = v.impl_->succ_chans;
-         j["type"] = static_cast<halide_type_t>(v.impl_->type);
-         j["dimensions"] = v.impl_->dimensions;
-         j["size"] = v.impl_->params.size();
-         j["impl_ptr"] = reinterpret_cast<uintptr_t>(v.impl_.get());
-         j["index"] = v.index_;
+         j["pred_chan"] = v.pred_chan();
+         j["succ_chans"] = v.succ_chans();
+         j["type"] = static_cast<halide_type_t>(v.type());
+         j["dimensions"] = v.dimensions();
+         j["size"] = v.size();
+         j["impl_ptr"] = v.impl_ptr();
+         j["index"] = v.index();
      }
 
      static void from_json(const json& j, ion::Port& v) {
-         v = ion::Port(ion::Port::find_impl(j["impl_ptr"].get<uintptr_t>()));
-         v.impl_->pred_chan = j["pred_chan"].get<ion::Port::Channel>();
-         v.impl_->succ_chans = j["succ_chans"].get<std::set<ion::Port::Channel>>();
-         v.impl_->type = j["type"].get<halide_type_t>();
-         v.impl_->dimensions = j["dimensions"];
-         for (auto i=0; i<j["size"]; ++i) {
-             v.impl_->params[i] = Halide::Internal::Parameter(v.impl_->type, v.impl_->dimensions != 0, v.impl_->dimensions, ion::argument_name(v.pred_id(), v.pred_name(), i));
+         auto [impl, found] = ion::Port::find_impl(j["impl_ptr"].get<uintptr_t>());
+         if (!found) {
+             impl->pred_chan = j["pred_chan"].get<ion::Port::Channel>();
+             impl->succ_chans = j["succ_chans"].get<std::set<ion::Port::Channel>>();
+             impl->type = j["type"].get<halide_type_t>();
+             impl->dimensions = j["dimensions"];
+             for (auto i=0; i<j["size"]; ++i) {
+                 impl->params[i] = Halide::Internal::Parameter(impl->type, impl->dimensions != 0, impl->dimensions,
+                                                               ion::argument_name(std::get<0>(impl->pred_chan), std::get<1>(impl->pred_chan), i));
+             }
          }
-         v.index_ = j["index"];
+         v = ion::Port(impl, j["index"]);
      }
 };
 
 template <>
 struct adl_serializer<ion::Node> {
      static void to_json(json& j, const ion::Node& v) {
-         j["id"] = v.impl_->id;
-         j["name"] = v.impl_->name;
-         j["target"] = v.impl_->target.to_string();
-         j["params"] = v.impl_->params;
-         j["ports"] = v.impl_->ports;
+         j["id"] = v.id();
+         j["name"] = v.name();
+         j["target"] = v.target().to_string();
+         j["params"] = v.params();
+         j["ports"] = v.ports();
      }
 
      static void from_json(const json& j, ion::Node& v) {
-         v.impl_->id = j["id"].get<std::string>();
-         v.impl_->name = j["name"].get<std::string>();
-         v.impl_->target = Halide::Target(j["target"].get<std::string>());
-         v.impl_->params = j["params"].get<std::vector<ion::Param>>();
-         v.impl_->ports = j["ports"].get<std::vector<ion::Port>>();
-         auto bb(Halide::Internal::GeneratorRegistry::create(v.impl_->name, Halide::GeneratorContext(v.impl_->target)));
+         auto impl = std::make_shared<ion::Node::Impl>();
+         impl->id = j["id"].get<std::string>();
+         impl->name = j["name"].get<std::string>();
+         impl->target = Halide::Target(j["target"].get<std::string>());
+         impl->params = j["params"].get<std::vector<ion::Param>>();
+         impl->ports = j["ports"].get<std::vector<ion::Port>>();
+         auto bb(Halide::Internal::GeneratorRegistry::create(impl->name, Halide::GeneratorContext(impl->target)));
          if (!bb) {
-             ion::log::error("BuildingBlock {} is not found", v.impl_->name);
+             ion::log::error("BuildingBlock {} is not found", impl->name);
              throw std::runtime_error("Failed to create building block object");
          }
-         v.impl_->arginfos = bb->arginfos();
+         impl->arginfos = bb->arginfos();
+         v = ion::Node(impl);
      }
 };
 }


Test patch:
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index a51e849b..cb84f929 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -62,7 +62,8 @@ endif()
 ion_jit_executable(dup-port-name SRCS dup-port-name.cc)
 
 # Export test
-ion_jit_executable(export SRCS export.cc)
+# TODO: Resolve defects in feature/win-debug branch on Windows environment
+# ion_jit_executable(export SRCS export.cc)
 
 ion_aot_executable(simple_graph SRCS_COMPILE simple_graph_compile.cc SRCS_RUN simple_graph_run.cc LIBS ion-bb-test)
 ion_jit_executable(simple_graph SRCS simple_graph_jit.cc)