From 09bc9cb1640b81aa77f43b7904733d1b0dfe3dbc Mon Sep 17 00:00:00 2001
From: Teon Banek <teon.banek@memgraph.io>
Date: Mon, 30 Jul 2018 15:57:19 +0200
Subject: [PATCH] Update save/load hooks in LCP

Reviewers: mtomic

Reviewed By: mtomic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D1517
---
 src/database/state_delta.lcp                  |   7 -
 src/distributed/bfs_rpc_messages.lcp          |  50 +----
 src/distributed/data_rpc_messages.lcp         |   8 +-
 src/distributed/plan_rpc_messages.lcp         |   5 +-
 src/distributed/pull_produce_rpc_messages.lcp | 182 +-----------------
 src/distributed/updates_rpc_messages.lcp      |  20 --
 src/lisp/lcp.lisp                             |  18 +-
 7 files changed, 21 insertions(+), 269 deletions(-)

diff --git a/src/database/state_delta.lcp b/src/database/state_delta.lcp
index 70a85879e..81272495e 100644
--- a/src/database/state_delta.lcp
+++ b/src/database/state_delta.lcp
@@ -50,13 +50,6 @@ cpp<#
    (property "storage::Property")
    (property-name "std::string")
    (value "PropertyValue" :initval "PropertyValue::Null"
-          :save-fun #>cpp utils::SaveTypedValue(ar, value); cpp<#
-          :load-fun
-          #>cpp
-          query::TypedValue tv;
-          utils::LoadTypedValue(ar, tv);
-          value = tv;
-          cpp<#
           :capnp-type "Dis.TypedValue"
           :capnp-save
           (lambda (builder member)
diff --git a/src/distributed/bfs_rpc_messages.lcp b/src/distributed/bfs_rpc_messages.lcp
index e96ad1359..033e234d6 100644
--- a/src/distributed/bfs_rpc_messages.lcp
+++ b/src/distributed/bfs_rpc_messages.lcp
@@ -57,54 +57,14 @@ cpp<#
   ((global-address "storage::Address<mvcc::VersionList<TElement>>"
                    :capnp-type "Storage.Address")
    (old-element-input "TElement *"
-                      :save-fun
-                      "if (old_element_input) {
-                         ar << true;
-                         SaveElement(ar, *old_element_input, worker_id);
-                       } else {
-                         ar << false;
-                       }"
-                      :load-fun ""
                       :capnp-type '((null "Void") (vertex "Dis.Vertex") (edge "Dis.Edge"))
                       :capnp-save #'save-element :capnp-load #'load-element)
-   (old-element-output "std::unique_ptr<TElement>"
-                       :save-fun ""
-                       :load-fun
-                       "bool has_old;
-                        ar >> has_old;
-                        if (has_old) {
-                          if constexpr (std::is_same<TElement, Vertex>::value) {
-                            old_element_output = std::move(LoadVertex(ar));
-                          } else {
-                            old_element_output = std::move(LoadEdge(ar));
-                          }
-                        }"
-                       :capnp-save :dont-save)
+   (old-element-output "std::unique_ptr<TElement>" :capnp-save :dont-save)
    (new-element-input "TElement *"
-                      :save-fun
-                      "if (new_element_input) {
-                         ar << true;
-                         SaveElement(ar, *new_element_input, worker_id);
-                       } else {
-                         ar << false;
-                       }"
-                      :load-fun ""
                       :capnp-type '((null "Void") (vertex "Dis.Vertex") (edge "Dis.Edge"))
                       :capnp-save #'save-element :capnp-load #'load-element)
-   (new-element-output "std::unique_ptr<TElement>"
-                       :save-fun ""
-                       :load-fun
-                       "bool has_new;
-                        ar >> has_new;
-                        if (has_new) {
-                          if constexpr (std::is_same<TElement, Vertex>::value) {
-                            new_element_output = std::move(LoadVertex(ar));
-                          } else {
-                            new_element_output = std::move(LoadEdge(ar));
-                          }
-                        }"
-                       :capnp-save :dont-save)
-   (worker-id :int16_t :save-fun "" :load-fun "" :capnp-save :dont-save))
+   (new-element-output "std::unique_ptr<TElement>" :capnp-save :dont-save)
+   (worker-id :int16_t :capnp-save :dont-save))
   (:public
    #>cpp
     SerializedGraphElement(storage::Address<mvcc::VersionList<TElement>> global_address,
@@ -146,7 +106,7 @@ cpp<#
                                                   '(in out both)))
       ;; TODO(mtomic): Why isn't edge-types serialized?
       (edge-types "std::vector<storage::EdgeType>"
-                  :save-fun "" :load-fun "" :capnp-save :dont-save)
+                  :capnp-save :dont-save)
       (graph-view "query::GraphView"
                   :capnp-type "Query.GraphView" :capnp-init nil
                   :capnp-save (lcp:capnp-save-enum "::query::capnp::GraphView"
@@ -244,7 +204,7 @@ cpp<#
       cpp<#))
   (:response
    ((subcursor-id :int64_t ;; TODO(mtomic): Unused?
-                  :save-fun "" :load-fun "" :capnp-save :dont-save)
+                  :capnp-save :dont-save)
     (edges "std::vector<SerializedEdge>" :capnp-type "List(SerializedGraphElement)"
            :capnp-save (lcp:capnp-save-vector "capnp::SerializedGraphElement" "SerializedEdge")
            :capnp-load (lcp:capnp-load-vector "capnp::SerializedGraphElement" "SerializedEdge"))
diff --git a/src/distributed/data_rpc_messages.lcp b/src/distributed/data_rpc_messages.lcp
index 2713ba61d..22b686615 100644
--- a/src/distributed/data_rpc_messages.lcp
+++ b/src/distributed/data_rpc_messages.lcp
@@ -29,7 +29,6 @@ cpp<#
     (:request ((member "TxGidPair")))
   (:response
    ((vertex-input "const Vertex *"
-                  :save-fun "SaveVertex(ar, *vertex_input, worker_id);" :load-fun ""
                   :capnp-type "Dist.Vertex"
                   :capnp-save
                   (lambda (builder member)
@@ -42,16 +41,14 @@ cpp<#
                     #>cpp
                     vertex_output = LoadVertex(${reader});
                     cpp<#))
-    (worker-id :int64_t :save-fun "" :load-fun "" :capnp-save :dont-save)
+    (worker-id :int64_t :capnp-save :dont-save)
     (vertex-output "std::unique_ptr<Vertex>" :initarg nil
-                   :save-fun "" :load-fun "vertex_output = LoadVertex(ar);"
                    :capnp-save :dont-save))))
 
 (lcp:define-rpc edge
     (:request ((member "TxGidPair")))
   (:response
    ((edge-input "const Edge *"
-                :save-fun "SaveEdge(ar, *edge_input, worker_id);" :load-fun ""
                 :capnp-type "Dist.Edge"
                 :capnp-save
                 (lambda (builder member)
@@ -64,9 +61,8 @@ cpp<#
                   #>cpp
                   edge_output = LoadEdge(${reader});
                   cpp<#))
-    (worker-id :int64_t :save-fun "" :load-fun "" :capnp-save :dont-save)
+    (worker-id :int64_t :capnp-save :dont-save)
     (edge-output "std::unique_ptr<Edge>" :initarg nil
-                 :save-fun "" :load-fun "edge_output = LoadEdge(ar);"
                  :capnp-save :dont-save))))
 
 (lcp:define-rpc vertex-count
diff --git a/src/distributed/plan_rpc_messages.lcp b/src/distributed/plan_rpc_messages.lcp
index bf2f892a5..d70ca8f4b 100644
--- a/src/distributed/plan_rpc_messages.lcp
+++ b/src/distributed/plan_rpc_messages.lcp
@@ -46,10 +46,7 @@ cpp<#
             :capnp-type "Utils.SharedPtr(Plan.LogicalOperator)"
             :capnp-save #'save-plan :capnp-load #'load-plan)
       (symbol-table "query::SymbolTable" :capnp-type "Sem.SymbolTable")
-      (storage "query::AstStorage" :initarg nil
-               :save-fun ""
-               :load-fun "storage = std::move(ar.template get_helper<query::AstStorage>(query::AstStorage::kHelperId));"
-               :capnp-save :dont-save)))
+      (storage "query::AstStorage" :initarg nil :capnp-save :dont-save)))
   (:response ()))
 
 (lcp:define-rpc remove-plan
diff --git a/src/distributed/pull_produce_rpc_messages.lcp b/src/distributed/pull_produce_rpc_messages.lcp
index 0c79d4c29..8a9d2c822 100644
--- a/src/distributed/pull_produce_rpc_messages.lcp
+++ b/src/distributed/pull_produce_rpc_messages.lcp
@@ -179,125 +179,6 @@ to the appropriate value. Not used on side that generates the response.")
     PullResData &operator=(const PullResData &) = delete;
     PullResData(PullResData &&) = default;
     PullResData &operator=(PullResData &&) = default;
-
-
-    /// Saves a typed value that is a vertex/edge/path.
-    template <class TArchive>
-    void SaveGraphElement(TArchive &ar, const query::TypedValue &value) const {
-      // Helper template function for storing a vertex or an edge.
-      auto save_element = [&ar, this](auto element_accessor) {
-        ar << element_accessor.GlobalAddress().raw();
-
-        // If both old and new are null, we need to reconstruct.
-        if (!(element_accessor.GetOld() || element_accessor.GetNew())) {
-          bool result = element_accessor.Reconstruct();
-          CHECK(result) << "Attempting to serialize an element not visible to "
-                           "current transaction.";
-        }
-        auto *old_rec = element_accessor.GetOld();
-        if (send_old && old_rec) {
-          ar << true;
-          distributed::SaveElement(ar, *old_rec, worker_id);
-        } else {
-          ar << false;
-        }
-        if (send_new) {
-          // Must call SwitchNew as that will trigger a potentially necesary
-          // Reconstruct.
-          element_accessor.SwitchNew();
-          auto *new_rec = element_accessor.GetNew();
-          if (new_rec) {
-            ar << true;
-            distributed::SaveElement(ar, *new_rec, worker_id);
-          } else {
-            ar << false;
-          }
-        } else {
-          ar << false;
-        }
-      };
-      switch (value.type()) {
-        case query::TypedValue::Type::Vertex:
-          save_element(value.ValueVertex());
-          break;
-        case query::TypedValue::Type::Edge:
-          save_element(value.ValueEdge());
-          break;
-        case query::TypedValue::Type::Path: {
-          auto &path = value.ValuePath();
-          ar << path.size();
-          save_element(path.vertices()[0]);
-          for (size_t i = 0; i < path.size(); ++i) {
-            save_element(path.edges()[i]);
-            save_element(path.vertices()[i + 1]);
-          }
-          break;
-        }
-        default:
-          LOG(FATAL) << "Unsupported graph element type: " << value.type();
-      }
-    }
-
-    /// Loads a typed value that is a vertex/edge/path. Part of the
-    /// deserialization process, populates the temporary data caches which are
-    /// processed later.
-    template <class TArchive>
-    void LoadGraphElement(TArchive &ar, query::TypedValue::Type type,
-                          query::TypedValue &value) {
-      auto load_edge = [](auto &ar) {
-        bool exists;
-        ar >> exists;
-        return exists ? LoadEdge(ar) : nullptr;
-      };
-      auto load_vertex = [](auto &ar) {
-        bool exists;
-        ar >> exists;
-        return exists ? LoadVertex(ar) : nullptr;
-      };
-
-      switch (type) {
-        case query::TypedValue::Type::Vertex: {
-          storage::VertexAddress::StorageT address;
-          ar >> address;
-          vertices.emplace_back(storage::VertexAddress(address), load_vertex(ar),
-                                load_vertex(ar), &value);
-          break;
-        }
-        case query::TypedValue::Type::Edge: {
-          storage::VertexAddress::StorageT address;
-          ar >> address;
-          edges.emplace_back(storage::EdgeAddress(address), load_edge(ar),
-                             load_edge(ar), &value);
-          break;
-        }
-        case query::TypedValue::Type::Path: {
-          size_t path_size;
-          ar >> path_size;
-
-          paths.emplace_back(&value);
-          auto &path_data = paths.back();
-
-          storage::VertexAddress::StorageT vertex_address;
-          storage::EdgeAddress::StorageT edge_address;
-          ar >> vertex_address;
-          path_data.vertices.emplace_back(storage::VertexAddress(vertex_address),
-                                          load_vertex(ar), load_vertex(ar),
-                                          nullptr);
-          for (size_t i = 0; i < path_size; ++i) {
-            ar >> edge_address;
-            path_data.edges.emplace_back(storage::EdgeAddress(edge_address),
-                                         load_edge(ar), load_edge(ar), nullptr);
-            ar >> vertex_address;
-            path_data.vertices.emplace_back(
-                storage::VertexAddress(vertex_address), load_vertex(ar),
-                load_vertex(ar), nullptr);
-          }
-          break;
-        }
-        default:
-          LOG(FATAL) << "Unsupported graph element type: " << type;
-      }
-    }
    cpp<#)
   (:private
    #>cpp
@@ -436,28 +317,6 @@ cpp<#)
       (plan-id :int64_t)
       (command-id "tx::CommandId")
       (params "Parameters"
-              :save-fun
-              "
-              ar << params.size();
-              for (auto &kv : params) {
-                ar << kv.first;
-                // Params never contain a vertex/edge, so save plan TypedValue.
-                utils::SaveTypedValue(ar, kv.second);
-              }
-              "
-              :load-fun
-              "
-              size_t params_size;
-              ar >> params_size;
-              for (size_t i = 0; i < params_size; ++i) {
-                int token_pos;
-                ar >> token_pos;
-                query::TypedValue param;
-                // Params never contain a vertex/edge, so load plan TypedValue.
-                utils::LoadTypedValue(ar, param);
-                params.Add(token_pos, param);
-              }
-              "
               :capnp-type "Utils.Map(Utils.BoxInt64, Dis.TypedValue)"
               :capnp-save
               (lambda (builder member)
@@ -490,46 +349,7 @@ cpp<#)
       (send-old :bool)
       (send-new :bool)))
   (:response
-   ((data "PullResData" :initarg :move
-          :save-fun
-          "
-          ar << data.pull_state;
-          ar << data.frames.size();
-          // We need to indicate how many values are in each frame.
-          // Assume all the frames have an equal number of elements.
-          ar << (data.frames.size() == 0 ? 0 : data.frames[0].size());
-          for (const auto &frame : data.frames) {
-            for (const auto &value : frame) {
-              utils::SaveTypedValue<TArchive>(
-                  ar, value, [this](TArchive &ar, const query::TypedValue &value) {
-                    data.SaveGraphElement(ar, value);
-                  });
-            }
-          }
-          "
-          :load-fun
-          "
-          ar >> data.pull_state;
-          size_t frame_count;
-          ar >> frame_count;
-          data.frames.reserve(frame_count);
-          size_t frame_size;
-          ar >> frame_size;
-          for (size_t i = 0; i < frame_count; ++i) {
-            data.frames.emplace_back();
-            auto &current_frame = data.frames.back();
-            current_frame.reserve(frame_size);
-            for (size_t j = 0; j < frame_size; ++j) {
-              current_frame.emplace_back();
-              utils::LoadTypedValue<TArchive>(
-                  ar, current_frame.back(),
-                  [this](TArchive &ar, query::TypedValue::TypedValue::Type type,
-                         query::TypedValue &value) {
-                    data.LoadGraphElement(ar, type, value);
-                  });
-            }
-          }
-          "))
+   ((data "PullResData" :initarg :move))
    (:serialize :capnp :base t :load-args '((dba "database::GraphDbAccessor *")
                                            (data-manager "distributed::DataManager *")))))
 
diff --git a/src/distributed/updates_rpc_messages.lcp b/src/distributed/updates_rpc_messages.lcp
index 7f875f528..84b6ff12f 100644
--- a/src/distributed/updates_rpc_messages.lcp
+++ b/src/distributed/updates_rpc_messages.lcp
@@ -66,26 +66,6 @@ cpp<#
      :capnp-save (lcp:capnp-save-vector "storage::capnp::Common" "storage::Label")
      :capnp-load (lcp:capnp-load-vector "storage::capnp::Common" "storage::Label"))
    (properties "std::unordered_map<storage::Property, query::TypedValue>"
-               :save-fun
-               #>cpp
-               ar << properties.size();
-               for (auto &kv : properties) {
-                 ar << kv.first;
-                 utils::SaveTypedValue(ar, kv.second);
-               }
-               cpp<#
-               :load-fun
-               #>cpp
-               size_t props_size;
-               ar >> props_size;
-               for (size_t i = 0; i < props_size; ++i) {
-                 storage::Property p;
-                 ar >> p;
-                 query::TypedValue tv;
-                 utils::LoadTypedValue(ar, tv);
-                 properties.emplace(p, std::move(tv));
-               }
-               cpp<#
                :capnp-type "Utils.Map(Storage.Common, Dis.TypedValue)"
                :capnp-save
                (lambda (builder member)
diff --git a/src/lisp/lcp.lisp b/src/lisp/lcp.lisp
index 7cb6183b6..9fd3690d4 100644
--- a/src/lisp/lcp.lisp
+++ b/src/lisp/lcp.lisp
@@ -219,14 +219,12 @@ produces:
   ;; TODO: Support giving a name for reader function.
   (reader nil :type boolean :read-only t)
   (documentation nil :type (or null string) :read-only t)
-  ;; Custom saving and loading code. May be a function which takes 2
-  ;; args: (archive member-name) and needs to return C++ code.
-  (save-fun nil :type (or null string raw-cpp function) :read-only t)
-  (load-fun nil :type (or null string raw-cpp function) :read-only t)
   ;; CAPNP-TYPE may be a string specifying the type, or a list of
   ;; (member-symbol "capnp-type") specifying a union type.
   (capnp-type nil :type (or null string list) :read-only t)
   (capnp-init t :type boolean :read-only t)
+  ;; Custom saving and loading code. May be a function which takes 2
+  ;; args: (builder-or-reader member-name) and needs to return C++ code.
   (capnp-save nil :type (or null function (eql :dont-save)) :read-only t)
   (capnp-load nil :type (or null function) :read-only t))
 
@@ -1319,8 +1317,16 @@ slot-options are keyword arguments. Currently supported options are:
   * :reader -- if t, generates a public getter for the member.
   * :scope -- class scope of the member, either :public, :protected or :private (default).
   * :documentation -- Doxygen documentation of the member.
-  * :save-fun -- Custom code for serializing this member.
-  * :load-fun -- Custom code for deserializing this member.
+  * :capnp-type -- String or list specifying which Cap'n Proto type to use for
+    serialization.  If a list of (member-symbol \"capnp-type\") then a union
+    type is specified.
+  * :capnp-init -- Boolean indicating whether the member needs to be
+    initialized in Cap'n Proto structure, by calling `builder.init<member>`.
+    This is T by default, you may need to set it to NIL if the LCP doesn't
+    correctly recognize a primitive type or you wish to call `init<member>`
+    yourself.
+  * :capnp-save -- Custom code for serializing this member.
+  * :capnp-load -- Custom code for deserializing this member.
 
 Currently supported class-options are:
   * :documentation -- Doxygen documentation of the class.