Hello Curtis Dunham,

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

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

to review the following change.


Change subject: sim: Add hooks to implement event reference counting
......................................................................

sim: Add hooks to implement event reference counting

We currently only support deleting an event if it is triggered and not
re-scheduled. This is fine for most native code. However, there are
cases where Python needs to count references to make sure that the
Python object stays live while the native object is live.

Generalise the mechanism used to implement AutoDelete to use a
reference manager that can be used to implement reference
counting. The reference manager has two calls:

  * RefManager::incref(T *o)
  * RefManager::decref(T *o)

These calls are used to increase and decrease the reference count of
an object. The default implementation in Event maintains backwards
compatibility with the existing AutoDelete feature by ignoring
incref() and deleting the event on decref() if it isn't scheduled
anymore.

Custom reference counting implementations can be implemented by
overriding the Event::getRefManager() method and returning a pointer
to a custom reference manager. Reference counting can then be enabled
by setting the Managed flag at Event creation.

Change-Id: I5637984c906a9d44c22780712cf1c521b8297149
Signed-off-by: Andreas Sandberg <[email protected]>
Reviewed-by: Curtis Dunham <[email protected]>
---
M src/base/refcnt.hh
M src/sim/eventq.cc
M src/sim/eventq.hh
M src/sim/eventq_impl.hh
4 files changed, 63 insertions(+), 7 deletions(-)



diff --git a/src/base/refcnt.hh b/src/base/refcnt.hh
index baf08c6..f5da1fb 100644
--- a/src/base/refcnt.hh
+++ b/src/base/refcnt.hh
@@ -37,6 +37,17 @@
  * Classes for managing reference counted objects.
  */

+
+template<typename T>
+class RefManager
+{
+  public:
+    virtual ~RefManager() {};
+
+    virtual void incref(T *o) const = 0;
+    virtual void decref(T *o) const = 0;
+};
+
 /**
  * Derive from RefCounted if you want to enable reference counting of
  * this class.  If you want to use automatic reference counting, you
diff --git a/src/sim/eventq.cc b/src/sim/eventq.cc
index 7c2648c..3694a28 100644
--- a/src/sim/eventq.cc
+++ b/src/sim/eventq.cc
@@ -49,6 +49,19 @@

 Tick simQuantum = 0;

+
+class EventAutoDelete : public RefManager<Event>
+{
+  public:
+    void incref(Event *e) const final {}
+    void decref(Event *e) const final {
+        if (!e->scheduled())
+            delete e;
+    }
+};
+
+static EventAutoDelete autoDeleter;
+
 //
 // Main Event Queues
 //
@@ -168,6 +181,12 @@
     return top;
 }

+const RefManager<Event> *
+Event::getRefManager() const
+{
+    return &autoDeleter;
+}
+
 void
 EventQueue::remove(Event *event)
 {
@@ -227,7 +246,7 @@

         event->process();
         if (event->isExitEvent()) {
-            assert(!event->flags.isSet(Event::AutoDelete) ||
+            assert(!event->flags.isSet(Event::Managed) ||
!event->flags.isSet(Event::IsMainQueue)); // would be silly
             return event;
         }
@@ -235,8 +254,7 @@
         event->flags.clear(Event::Squashed);
     }

-    if (event->flags.isSet(Event::AutoDelete) && !event->scheduled())
-        delete event;
+    EventQueue::decref(event);

     return NULL;
 }
diff --git a/src/sim/eventq.hh b/src/sim/eventq.hh
index 95a36ca..4874aba 100644
--- a/src/sim/eventq.hh
+++ b/src/sim/eventq.hh
@@ -47,6 +47,7 @@
 #include <string>

 #include "base/flags.hh"
+#include "base/refcnt.hh"
 #include "base/types.hh"
 #include "debug/Event.hh"
 #include "sim/serialize.hh"
@@ -99,7 +100,8 @@
     static const FlagsType PublicWrite   = 0x001d; // public writable flags
     static const FlagsType Squashed      = 0x0001; // has been squashed
     static const FlagsType Scheduled     = 0x0002; // has been scheduled
-    static const FlagsType AutoDelete    = 0x0004; // delete after dispatch
+ static const FlagsType Managed = 0x0004; // Use life cycle manager + static const FlagsType AutoDelete = Managed; // delete after dispatch
     /**
      * This used to be AutoSerialize. This value can't be reused
      * without changing the checkpoint version since the flag field
@@ -282,6 +284,19 @@
     // This function isn't really useful if TRACING_ON is not defined
     virtual void trace(const char *action);     //!< trace event activity

+    /**
+     * Get a pointer to the reference manager that handles this event
+     *
+     * The reference manager implements generalized support for
+     * automatic event deletion. The default reference manager ignores
+     * RefManager::incref() calls and deletes an event on
+     * RefManager::decref() if the object isn't scheduled anymore.
+     *
+     * Objects that are exposed to managed languages will need to
+     * override this and implement proper reference counting.
+     */
+    virtual const RefManager<Event> *getRefManager() const;
+
   public:

     /*
@@ -340,7 +355,8 @@
     bool isExitEvent() const { return flags.isSet(IsExitEvent); }

     /// Check whether this event will auto-delete
-    bool isAutoDelete() const { return flags.isSet(AutoDelete); }
+    bool isManaged() const { return flags.isSet(Managed); }
+    bool isAutoDelete() const { return isManaged(); }

     /// Get the time that the event is scheduled
     Tick when() const { return _when; }
@@ -479,6 +495,17 @@
     //! owning thread, should call this function instead of insert().
     void asyncInsert(Event *event);

+    static void incref(Event *event)
+    {
+        if (event->flags.isSet(Event::Managed))
+            event->getRefManager()->incref(event);
+    }
+
+    static void decref(Event *event) {
+        if (event->flags.isSet(Event::Managed))
+            event->getRefManager()->decref(event);
+    }
+
     EventQueue(const EventQueue &);

   public:
diff --git a/src/sim/eventq_impl.hh b/src/sim/eventq_impl.hh
index 360731d..e82c854 100644
--- a/src/sim/eventq_impl.hh
+++ b/src/sim/eventq_impl.hh
@@ -45,6 +45,7 @@
     assert(!event->scheduled());
     assert(event->initialized());

+    EventQueue::incref(event);
     event->setWhen(when, this);

     // The check below is to make sure of two things
@@ -79,8 +80,7 @@
     if (DTRACE(Event))
         event->trace("descheduled");

-    if (event->flags.isSet(Event::AutoDelete))
-        delete event;
+    EventQueue::decref(event);
 }

 inline void

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

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5637984c906a9d44c22780712cf1c521b8297149
Gerrit-Change-Number: 3221
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Curtis Dunham <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to