Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/49410 )

Change subject: python,sim: Change how the m5.* importer code is integrated.
......................................................................

python,sim: Change how the m5.* importer code is integrated.

Previously, the importer module was built into gem5 as a compressed
bytecode blob like all the other code, and it had to be singled out and
installed manually so that it could help bring in all the other modules.
That adds some amount of complexity since it has to be identified and
treated as a special case.

Instead, this change builds it into gem5 using pybind11's
PYBIND11_EMBEDDED_MODULE macro, and a string that gets evaluated into
the new module's __dict__. This means the importer module is
automatically available just by building in that .cc, and it can just be
imported to start using it.

Theoretically all the embedded python could be handled this way, but
that would mean building it into gem5 as raw strings which wouldn't even
be compiled into byte code until run time. That would take more space in
the binary, and also delay catching simple errors.

Change-Id: Ic600bf6bce41a53289a2833484a655dd5a226e03
---
M src/python/SConscript
A src/python/importer.cc
D src/python/importer.py
M src/sim/init.cc
M src/sim/init.hh
5 files changed, 93 insertions(+), 102 deletions(-)



diff --git a/src/python/SConscript b/src/python/SConscript
index 37e83f9..e8937ad 100644
--- a/src/python/SConscript
+++ b/src/python/SConscript
@@ -28,7 +28,6 @@

 Import('*')

-PySource('', 'importer.py')
 PySource('m5', 'm5/__init__.py')
 PySource('m5', 'm5/SimObject.py')
 PySource('m5', 'm5/config.py')
@@ -71,6 +70,8 @@
 PySource('m5.ext.pystats', 'm5/ext/pystats/jsonloader.py')
 PySource('m5.stats', 'm5/stats/gem5stats.py')

+Source('importer.cc', add_tags='python')
+
 Source('pybind11/core.cc', add_tags='python')
 Source('pybind11/debug.cc', add_tags='python')
 Source('pybind11/event.cc', add_tags='python')
diff --git a/src/python/importer.cc b/src/python/importer.cc
new file mode 100644
index 0000000..39c4871
--- /dev/null
+++ b/src/python/importer.cc
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2008 The Hewlett-Packard Development Company
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "pybind11/embed.h"
+#include "pybind11/pybind11.h"
+
+namespace py = pybind11;
+
+PYBIND11_EMBEDDED_MODULE(importer, m)
+{
+    py::exec(R"(
+    import importlib
+    import os
+
+    class ByteCodeLoader(importlib.abc.Loader):
+        def __init__(self, code):
+            super(ByteCodeLoader, self).__init__()
+            self.code = code
+
+        def exec_module(self, module):
+            exec(self.code, module.__dict__)
+
+    # Simple importer that allows python to import data from a dict of
+    # code objects.  The keys are the module path, and the items are the
+    # filename and bytecode of the file.
+    class CodeImporter(object):
+        def __init__(self):
+            self.modules = {}
+
+        def add_module(self, abspath, modpath, code):
+            if modpath in self.modules:
+ raise AttributeError("%s already found in importer" % modpath)
+
+            self.modules[modpath] = (abspath, code)
+
+        def find_spec(self, fullname, path, target=None):
+            if fullname not in self.modules:
+                return None
+
+            abspath, code = self.modules[fullname]
+
+            is_package = (os.path.basename(abspath) == '__init__.py')
+            spec = importlib.util.spec_from_loader(
+                    name=fullname, loader=ByteCodeLoader(code),
+                    is_package=is_package)
+
+            spec.loader_state = self.modules.keys()
+
+            return spec
+
+    # Create an importer and add it to the meta_path so future imports can
+    # use it.  There's currently nothing in the importer, but calls to
+    # add_module can be used to add code.
+    import sys
+    importer = CodeImporter()
+    add_module = importer.add_module
+    sys.meta_path.append(importer)
+    )", m.attr("__dict__"));
+}
diff --git a/src/python/importer.py b/src/python/importer.py
deleted file mode 100644
index 7a2c8ff..0000000
--- a/src/python/importer.py
+++ /dev/null
@@ -1,72 +0,0 @@
-# Copyright (c) 2008 The Hewlett-Packard Development Company
-# All rights reserved.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions are
-# met: redistributions of source code must retain the above copyright
-# notice, this list of conditions and the following disclaimer;
-# redistributions in binary form must reproduce the above copyright
-# notice, this list of conditions and the following disclaimer in the
-# documentation and/or other materials provided with the distribution;
-# neither the name of the copyright holders nor the names of its
-# contributors may be used to endorse or promote products derived from
-# this software without specific prior written permission.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-import importlib
-import os
-
-class ByteCodeLoader(importlib.abc.Loader):
-    def __init__(self, code):
-        super(ByteCodeLoader, self).__init__()
-        self.code = code
-
-    def exec_module(self, module):
-        exec(self.code, module.__dict__)
-
-# Simple importer that allows python to import data from a dict of
-# code objects.  The keys are the module path, and the items are the
-# filename and bytecode of the file.
-class CodeImporter(object):
-    def __init__(self):
-        self.modules = {}
-
-    def add_module(self, abspath, modpath, code):
-        if modpath in self.modules:
-            raise AttributeError("%s already found in importer" % modpath)
-
-        self.modules[modpath] = (abspath, code)
-
-    def find_spec(self, fullname, path, target=None):
-        if fullname not in self.modules:
-            return None
-
-        abspath, code = self.modules[fullname]
-
-        is_package = (os.path.basename(abspath) == '__init__.py')
-        spec = importlib.util.spec_from_loader(
-                name=fullname, loader=ByteCodeLoader(code),
-                is_package=is_package)
-
-        spec.loader_state = self.modules.keys()
-
-        return spec
-
-# Create an importer and add it to the meta_path so future imports can
-# use it.  There's currently nothing in the importer, but calls to
-# add_module can be used to add code.
-import sys
-importer = CodeImporter()
-add_module = importer.add_module
-sys.meta_path.append(importer)
diff --git a/src/sim/init.cc b/src/sim/init.cc
index b6ba35e..6ba4146 100644
--- a/src/sim/init.cc
+++ b/src/sim/init.cc
@@ -73,18 +73,11 @@
 // so make a simple macro to make life a little easier
 #define PyCC(x) (const_cast<char *>(x))

-EmbeddedPython *EmbeddedPython::importer = NULL;
-PyObject *EmbeddedPython::importerModule = NULL;
 EmbeddedPython::EmbeddedPython(const char *abspath, const char *modpath,
         const unsigned char *code, int zlen, int len)
     : abspath(abspath), modpath(modpath), code(code), zlen(zlen), len(len)
 {
-    // if we've added the importer keep track of it because we need it
-    // to bootstrap.
-    if (std::string(modpath) == std::string("importer"))
-        importer = this;
-    else
-        getList().push_back(this);
+    getList().push_back(this);
 }

 std::list<EmbeddedPython *> &
@@ -114,15 +107,11 @@
 bool
 EmbeddedPython::addModule() const
 {
-    PyObject *code = getCode();
- PyObject *result = PyObject_CallMethod(importerModule, PyCC("add_module"),
-        PyCC("ssO"), abspath, modpath, code);
-    if (!result) {
-        PyErr_Print();
-        return false;
-    }
-
-    Py_DECREF(result);
+    auto code = py::reinterpret_borrow<py::object>(getCode());
+    // Ensure that "code" is not garbage collected.
+    code.inc_ref();
+    auto importer = py::module_::import("importer");
+    importer.attr("add_module")(abspath, modpath, code);
     return true;
 }

@@ -132,16 +121,7 @@
 int
 EmbeddedPython::initAll()
 {
-    // Load the importer module
-    PyObject *code = importer->getCode();
-    importerModule = PyImport_ExecCodeModule(PyCC("importer"), code);
-    if (!importerModule) {
-        PyErr_Print();
-        return 1;
-    }
-
-    // Load the rest of the embedded python files into the embedded
-    // python importer
+    // Load the embedded python files into the embedded python importer.
     for (auto *embedded: getList()) {
         if (!embedded->addModule())
             return 1;
diff --git a/src/sim/init.hh b/src/sim/init.hh
index c2f4cf2..9d73015 100644
--- a/src/sim/init.hh
+++ b/src/sim/init.hh
@@ -74,8 +74,6 @@
     PyObject *getCode() const;
     bool addModule() const;

-    static EmbeddedPython *importer;
-    static PyObject *importerModule;
     static std::list<EmbeddedPython *> &getList();
     static int initAll();
 };

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49410
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: Ic600bf6bce41a53289a2833484a655dd5a226e03
Gerrit-Change-Number: 49410
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
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

Reply via email to