[gem5-dev] Change in public/gem5[master]: python: Fix PyEvent reference counting bug

2017-05-24 Thread Andreas Sandberg (Gerrit) 
Andreas Sandberg has submitted this change and it was merged. (  
https://gem5-review.googlesource.com/3222 )


Change subject: python: Fix PyEvent reference counting bug
..

python: Fix PyEvent reference counting bug

The current implementation of reference counting for PyEvents only
partially works. The native object is currently kept alive while it is
in the event queue. However, if the Python object goes out of scope,
the Python side of this object is garbage collected which leaves a
"dangling" native object. This results in confusing error messages
where PyBind is unable to find the Python implementation of an event
when it is triggered.

Implement reference counting using the generalized reference counting
API instead.

Change-Id: I4e8e04abc4f61dff238d718065f5371e73b38ab3
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
Reviewed-on: https://gem5-review.googlesource.com/3222
Reviewed-by: Jason Lowe-Power 
---
M src/python/m5/event.py
M src/python/pybind11/event.cc
2 files changed, 44 insertions(+), 46 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Andreas Sandberg: Looks good to me, approved



diff --git a/src/python/m5/event.py b/src/python/m5/event.py
index 20f81b9..59d18b6 100644
--- a/src/python/m5/event.py
+++ b/src/python/m5/event.py
@@ -44,7 +44,8 @@
 import _m5.event

 from _m5.event import GlobalSimLoopExitEvent as SimExit
-from _m5.event import Event, getEventQueue, setEventQueue
+from _m5.event import PyEvent as Event
+from _m5.event import getEventQueue, setEventQueue

 mainq = None

diff --git a/src/python/pybind11/event.cc b/src/python/pybind11/event.cc
index 45a2d46..f9e6568 100644
--- a/src/python/pybind11/event.cc
+++ b/src/python/pybind11/event.cc
@@ -46,6 +46,7 @@
 #include "pybind11/pybind11.h"
 #include "pybind11/stl.h"

+#include "base/misc.hh"
 #include "sim/eventq.hh"
 #include "sim/sim_events.hh"
 #include "sim/sim_exit.hh"
@@ -53,6 +54,7 @@

 namespace py = pybind11;

+
 /**
  * PyBind wrapper for Events
  *
@@ -61,47 +63,42 @@
  * C++ cousin, PyEvents need to override __call__ instead of
  * Event::process().
  *
- * Memory management is mostly done using reference counting in
- * Python. However, PyBind can't keep track of the reference the event
- * queue holds to a scheduled event. We therefore need to inhibit
- * deletion and hand over ownership to the event queue in case a
- * scheduled event loses all of its Python references.
+ * Memory management is done using reference counting in Python.
  */
 class PyEvent : public Event
 {
   public:
-struct Deleter {
-void operator()(PyEvent *ev) {
-assert(!ev->isAutoDelete());
-if (ev->scheduled()) {
-// The event is scheduled, give ownership to the event
-// queue.
-ev->setFlags(Event::AutoDelete);
-} else {
-// The event isn't scheduled, hence Python owns it and
-// we need to free it here.
-delete ev;
-}
-}
-};
-
 PyEvent(Event::Priority priority)
-: Event(priority)
-{ }
+: Event(priority, Event::Managed)
+{
+}

 void process() override {
-if (isAutoDelete()) {
-// Ownership of the event was handed over to the event queue
-// because the last revference in Python land was GCed. We
-// need to claim the object again since we're creating a new
-// Python reference.
-clearFlags(AutoDelete);
-}
-
 // Call the Python implementation as __call__. This provides a
 // slightly more Python-friendly interface.
 PYBIND11_OVERLOAD_PURE_NAME(void, PyEvent, "__call__", process);
 }
+
+  protected:
+void acquireImpl() override {
+py::object obj = py::cast(this);
+
+if (obj) {
+obj.inc_ref();
+} else {
+panic("Failed to get PyBind object to increase ref count\n");
+}
+}
+
+void releaseImpl() override {
+py::object obj = py::cast(this);
+
+if (obj) {
+obj.dec_ref();
+} else {
+panic("Failed to get PyBind object to decrease ref count\n");
+}
+}
 };

 void
@@ -124,17 +121,15 @@
 .def("schedule", [](EventQueue *eq, PyEvent *e, Tick t) {
 eq->schedule(e, t);
 }, py::arg("event"), py::arg("when"))
-.def("deschedule", [](EventQueue *eq, PyEvent *e) {
-eq->deschedule(e);
-}, py::arg("event"))
-.def("reschedule", [](EventQueue *eq, PyEvent *e, Tick t, bool  
alw) {

-eq->reschedule(e, t, alw);
-}, py::arg("event"), py::arg("tick"), py::arg("always") =  
false)

+.def("deschedule", &EventQueue::deschedule,
+ py::arg("event"))
+.def("reschedule", &EventQueue::reschedule,
+

[gem5-dev] Change in public/gem5[master]: python: Fix PyEvent reference counting bug

2017-05-22 Thread Andreas Sandberg (Gerrit) 

Hello Gabe Black, Jason Lowe-Power, Nathan Binkert, Curtis Dunham,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/3222

to look at the new patch set (#3).

Change subject: python: Fix PyEvent reference counting bug
..

python: Fix PyEvent reference counting bug

The current implementation of reference counting for PyEvents only
partially works. The native object is currently kept alive while it is
in the event queue. However, if the Python object goes out of scope,
the Python side of this object is garbage collected which leaves a
"dangling" native object. This results in confusing error messages
where PyBind is unable to find the Python implementation of an event
when it is triggered.

Implement reference counting using the generalized reference counting
API instead.

Change-Id: I4e8e04abc4f61dff238d718065f5371e73b38ab3
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
---
M src/python/m5/event.py
M src/python/pybind11/event.cc
2 files changed, 44 insertions(+), 46 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/3222
To unsubscribe, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4e8e04abc4f61dff238d718065f5371e73b38ab3
Gerrit-Change-Number: 3222
Gerrit-PatchSet: 3
Gerrit-Owner: Andreas Sandberg 
Gerrit-Reviewer: Curtis Dunham 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nathan Binkert 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: python: Fix PyEvent reference counting bug

2017-05-17 Thread Andreas Sandberg (Gerrit) 

Hello Gabe Black, Jason Lowe-Power, Nathan Binkert, Curtis Dunham,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/3222

to look at the new patch set (#2).

Change subject: python: Fix PyEvent reference counting bug
..

python: Fix PyEvent reference counting bug

The current implementation of reference counting for PyEvents only
partially works. The native object is currently kept alive while it is
in the event queue. However, if the Python object goes out of scope,
the Python side of this object is garbage collected which leaves a
"dangling" native object. This results in confusing error messages
where PyBind is unable to find the Python implementation of an event
when it is triggered.

Implement reference counting using the generalized reference counting
API instead.

Change-Id: I4e8e04abc4f61dff238d718065f5371e73b38ab3
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
---
M src/python/m5/event.py
M src/python/pybind11/event.cc
2 files changed, 46 insertions(+), 46 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/3222
To unsubscribe, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4e8e04abc4f61dff238d718065f5371e73b38ab3
Gerrit-Change-Number: 3222
Gerrit-PatchSet: 2
Gerrit-Owner: Andreas Sandberg 
Gerrit-Reviewer: Curtis Dunham 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nathan Binkert 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: python: Fix PyEvent reference counting bug

2017-05-10 Thread Andreas Sandberg (Gerrit) 

Hello Curtis Dunham,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/3222

to review the following change.


Change subject: python: Fix PyEvent reference counting bug
..

python: Fix PyEvent reference counting bug

The current implementation of reference counting for PyEvents only
partially works. The native object is currently kept alive while it is
in the event queue. However, if the Python object goes out of scope,
the Python side of this object is garbage collected which leaves a
"dangling" native object. This results in confusing error messages
where PyBind is unable to find the Python implementation of an event
when it is triggered.

Implement reference counting using the generalized reference counting
API instead.

Change-Id: I4e8e04abc4f61dff238d718065f5371e73b38ab3
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
---
M src/python/m5/event.py
M src/python/pybind11/event.cc
2 files changed, 57 insertions(+), 46 deletions(-)



diff --git a/src/python/m5/event.py b/src/python/m5/event.py
index 20f81b9..59d18b6 100644
--- a/src/python/m5/event.py
+++ b/src/python/m5/event.py
@@ -44,7 +44,8 @@
 import _m5.event

 from _m5.event import GlobalSimLoopExitEvent as SimExit
-from _m5.event import Event, getEventQueue, setEventQueue
+from _m5.event import PyEvent as Event
+from _m5.event import getEventQueue, setEventQueue

 mainq = None

diff --git a/src/python/pybind11/event.cc b/src/python/pybind11/event.cc
index 45a2d46..90e8c3c 100644
--- a/src/python/pybind11/event.cc
+++ b/src/python/pybind11/event.cc
@@ -46,6 +46,7 @@
 #include "pybind11/pybind11.h"
 #include "pybind11/stl.h"

+#include "base/misc.hh"
 #include "sim/eventq.hh"
 #include "sim/sim_events.hh"
 #include "sim/sim_exit.hh"
@@ -53,6 +54,34 @@

 namespace py = pybind11;

+
+template
+class PyBindRefCnt : public RefManager
+{
+  public:
+void incref(T *o) const final {
+py::object obj = py::cast(o);
+
+if (obj) {
+obj.inc_ref();
+} else {
+panic("Failed to get PyBind object to increase ref count\n");
+}
+}
+
+void decref(T *o) const final {
+py::object obj = py::cast(o);
+
+if (obj) {
+obj.dec_ref();
+} else {
+panic("Failed to get PyBind object to decrease ref count\n");
+}
+}
+};
+
+static PyBindRefCnt pyEventManager;
+
 /**
  * PyBind wrapper for Events
  *
@@ -61,47 +90,28 @@
  * C++ cousin, PyEvents need to override __call__ instead of
  * Event::process().
  *
- * Memory management is mostly done using reference counting in
- * Python. However, PyBind can't keep track of the reference the event
- * queue holds to a scheduled event. We therefore need to inhibit
- * deletion and hand over ownership to the event queue in case a
- * scheduled event loses all of its Python references.
+ * Memory management is done using reference counting in Python. A
+ * custom reference manager is used to increase/decrease the ref count
+ * on the Python object when the event queue schedules new events.
  */
 class PyEvent : public Event
 {
   public:
-struct Deleter {
-void operator()(PyEvent *ev) {
-assert(!ev->isAutoDelete());
-if (ev->scheduled()) {
-// The event is scheduled, give ownership to the event
-// queue.
-ev->setFlags(Event::AutoDelete);
-} else {
-// The event isn't scheduled, hence Python owns it and
-// we need to free it here.
-delete ev;
-}
-}
-};
-
 PyEvent(Event::Priority priority)
-: Event(priority)
-{ }
+: Event(priority, Event::Managed)
+{
+}

 void process() override {
-if (isAutoDelete()) {
-// Ownership of the event was handed over to the event queue
-// because the last revference in Python land was GCed. We
-// need to claim the object again since we're creating a new
-// Python reference.
-clearFlags(AutoDelete);
-}
-
 // Call the Python implementation as __call__. This provides a
 // slightly more Python-friendly interface.
 PYBIND11_OVERLOAD_PURE_NAME(void, PyEvent, "__call__", process);
 }
+
+  protected:
+const RefManager *getRefManager() const override {
+return &pyEventManager;
+}
 };

 void
@@ -124,17 +134,15 @@
 .def("schedule", [](EventQueue *eq, PyEvent *e, Tick t) {
 eq->schedule(e, t);
 }, py::arg("event"), py::arg("when"))
-.def("deschedule", [](EventQueue *eq, PyEvent *e) {
-eq->deschedule(e);
-}, py::arg("event"))
-.def("reschedule", [](EventQueue *eq, PyEvent *e, Tick t, bool  
alw) {

-eq->reschedule(e, t, alw);
-}, py::arg("event"), py::a