[gem5-dev] Change in gem5/gem5[develop]: sim: Remove SimObject dependency from serialize.hh

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/38740 )

Change subject: sim: Remove SimObject dependency from serialize.hh
......................................................................
sim: Remove SimObject dependency from serialize.hh

Previously Serializable::serializeAll called SimObject::
serializeAll. This created an unnecessary dependency. This
change makes Serializable responsible for the generation
of the checkpoint file, and then the SimObjects will
perform the serialization of the object using that file.

With this change serialize.hh contains only functions
related to the (un)serialization of basic types or
objects that inherit from Serializable. As a general
rule, functions related to the (un)serialization of
specific/other types must be defined in the file that
introduces that type.

Change-Id: I9438b799d7e9d4c992a62c7f9d1f15f3f3250a5a
Signed-off-by: Daniel R. Carvalho <oda...@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38740
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Reviewed-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Maintainer: Jason Lowe-Power <power...@gmail.com>
---
M src/python/pybind11/core.cc
M src/sim/serialize.cc
M src/sim/serialize.hh
M src/sim/sim_object.cc
M src/sim/sim_object.hh
5 files changed, 62 insertions(+), 50 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, but someone else must approve; Looks  
good to me, approved
  Giacomo Travaglini: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/python/pybind11/core.cc b/src/python/pybind11/core.cc
index 3b99ede..a000ccc 100644
--- a/src/python/pybind11/core.cc
+++ b/src/python/pybind11/core.cc
@@ -307,7 +307,7 @@
      * Serialization helpers
      */
     m_core
-        .def("serializeAll", &Serializable::serializeAll)
+        .def("serializeAll", &SimObject::serializeAll)
         .def("getCheckpoint", [](const std::string &cpt_dir) {
             SimObject::setSimObjectResolver(&pybindSimObjectResolver);
             return new CheckpointIn(cpt_dir);
diff --git a/src/sim/serialize.cc b/src/sim/serialize.cc
index 86aaecb..93eaf2c 100644
--- a/src/sim/serialize.cc
+++ b/src/sim/serialize.cc
@@ -46,21 +46,12 @@
 #include <sys/time.h>
 #include <sys/types.h>

+#include <cassert>
 #include <cerrno>
-#include <fstream>
-#include <list>
-#include <set>
-#include <string>
-#include <vector>

 #include "base/inifile.hh"
-#include "base/output.hh"
 #include "base/trace.hh"
 #include "debug/Checkpoint.hh"
-#include "sim/sim_object.hh"
-
-// For stat reset hack
-#include "sim/stat_control.hh"

 int ckptMaxCount = 0;
 int ckptCount = 0;
@@ -92,20 +83,19 @@
 }

 void
-Serializable::serializeAll(const std::string &cpt_dir)
+Serializable::generateCheckpointOut(const std::string &cpt_dir,
+        std::ofstream &outstream)
 {
     std::string dir = CheckpointIn::setDir(cpt_dir);
     if (mkdir(dir.c_str(), 0775) == -1 && errno != EEXIST)
             fatal("couldn't mkdir %s\n", dir);

     std::string cpt_file = dir + CheckpointIn::baseFilename;
-    std::ofstream outstream(cpt_file.c_str());
+    outstream = std::ofstream(cpt_file.c_str());
     time_t t = time(NULL);
-    if (!outstream.is_open())
+    if (!outstream)
         fatal("Unable to open file %s for writing\n", cpt_file.c_str());
     outstream << "## checkpoint generated: " << ctime(&t);
-
-    SimObject::serializeAll(outstream);
 }

 Serializable::ScopedCheckpointSection::~ScopedCheckpointSection()
@@ -220,20 +210,3 @@
 {
     db->visitSection(section, cb);
 }
-
-void
-objParamIn(CheckpointIn &cp, const std::string &name, SimObject * &param)
-{
-    const std::string &section(Serializable::currentSection());
-    std::string path;
-    if (!cp.find(section, name, path)) {
-        fatal("Can't unserialize '%s:%s'\n", section, name);
-    }
-    param = SimObject::getSimObjectResolver()->resolveSimObject(path);
-}
-
-void
-debug_serialize(const std::string &cpt_dir)
-{
-    Serializable::serializeAll(cpt_dir);
-}
diff --git a/src/sim/serialize.hh b/src/sim/serialize.hh
index d20d9cf..5bd59a4 100644
--- a/src/sim/serialize.hh
+++ b/src/sim/serialize.hh
@@ -47,10 +47,11 @@


 #include <algorithm>
+#include <fstream>
 #include <iostream>
 #include <iterator>
 #include <stack>
-#include <set>
+#include <string>
 #include <type_traits>
 #include <unordered_map>
 #include <vector>
@@ -60,7 +61,6 @@
 #include "sim/serialize_handlers.hh"

 class IniFile;
-class SimObject;

 typedef std::ostream CheckpointOut;

@@ -301,11 +301,15 @@
     static const std::string &currentSection();

     /**
-     * Serializes all the SimObjects.
+     * Generate a checkpoint file so that the serialization can be routed  
to
+     * it.
      *
+     * @param cpt_dir The dir at which the cpt file will be created.
+     * @param outstream The cpt file.
      * @ingroup api_serialize
      */
-    static void serializeAll(const std::string &cpt_dir);
+    static void generateCheckpointOut(const std::string &cpt_dir,
+        std::ofstream &outstream);

   private:
     static std::stack<std::string> path;
@@ -499,16 +503,6 @@
     arrayParamIn<T>(cp, name, insert_it, size);
 }

-void
-debug_serialize(const std::string &cpt_dir);
-
-
-/**
- * @ingroup api_serialize
- */
-void
-objParamIn(CheckpointIn &cp, const std::string &name, SimObject * &param);
-
 /**
  * Serialize a mapping represented as two arrays: one containing names
  * and the other containing values.
@@ -643,6 +637,12 @@
 /**
  * \def SERIALIZE_OBJ(obj)
  *
+ * This macro serializes an object into its own section. The object must
+ * inherit from Serializable, but NOT from SimObject (i.e, it is an object
+ * in the strict sense of "object oriented programing"). Objects that
+ * derive from SimObject are automatically serialized elsewhere
+ * (@see Serializable, SimObject::serializeAll()).
+ *
  * @ingroup api_serialize
  */
 #define SERIALIZE_OBJ(obj) obj.serializeSection(cp, #obj)
diff --git a/src/sim/sim_object.cc b/src/sim/sim_object.cc
index 4c8d87b..949a990 100644
--- a/src/sim/sim_object.cc
+++ b/src/sim/sim_object.cc
@@ -129,8 +129,11 @@
 // static function: serialize all SimObjects.
 //
 void
-SimObject::serializeAll(CheckpointOut &cp)
+SimObject::serializeAll(const std::string &cpt_dir)
 {
+    std::ofstream cp;
+    Serializable::generateCheckpointOut(cpt_dir, cp);
+
     SimObjectList::reverse_iterator ri = simObjectList.rbegin();
     SimObjectList::reverse_iterator rend = simObjectList.rend();

@@ -194,3 +197,20 @@
     assert(_objNameResolver);
     return _objNameResolver;
 }
+
+void
+objParamIn(CheckpointIn &cp, const std::string &name, SimObject * &param)
+{
+    const std::string &section(Serializable::currentSection());
+    std::string path;
+    if (!cp.find(section, name, path)) {
+        fatal("Can't unserialize '%s:%s'\n", section, name);
+    }
+    param = SimObject::getSimObjectResolver()->resolveSimObject(path);
+}
+
+void
+debug_serialize(const std::string &cpt_dir)
+{
+    SimObject::serializeAll(cpt_dir);
+}
diff --git a/src/sim/sim_object.hh b/src/sim/sim_object.hh
index dad2217..a47b0a3 100644
--- a/src/sim/sim_object.hh
+++ b/src/sim/sim_object.hh
@@ -318,9 +318,18 @@
     void unserialize(CheckpointIn &cp) override {};

     /**
-     * Serialize all SimObjects in the system.
+     * Create a checkpoint by serializing all SimObjects in the system.
+     *
+     * This is the entry point in the process of checkpoint creation,
+     * so it will create the checkpoint file and then unfold into
+     * the serialization of all the sim objects declared.
+     *
+     * Each SimObject instance is explicitly and individually serialized
+     * in its own section. As such, the serialization functions should not
+     * be called on sim objects anywhere else; otherwise, these objects
+     * would be needlessly serialized more than once.
      */
-    static void serializeAll(CheckpointOut &cp);
+    static void serializeAll(const std::string &cpt_dir);

 #ifdef DEBUG
   public:
@@ -392,4 +401,14 @@
 void debugObjectBreak(const char *objs);
 #endif

+/**
+ * To avoid circular dependencies the unserialization of SimObjects must be
+ * implemented here.
+ *
+ * @ingroup api_serialize
+ */
+void objParamIn(CheckpointIn &cp, const std::string &name, SimObject *  
&param);
+
+void debug_serialize(const std::string &cpt_dir);
+
 #endif // __SIM_OBJECT_HH__



17 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38740
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I9438b799d7e9d4c992a62c7f9d1f15f3f3250a5a
Gerrit-Change-Number: 38740
Gerrit-PatchSet: 19
Gerrit-Owner: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Ciro Santilli <ciro.santi...@arm.com>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Hoa Nguyen <hoangu...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s