Modified finaliser to collect subregions

src/interpreter/object.cc

@@ -30,10 +30,12 @@ namespace verona::interpreter
     // skip it. In that case, there are also obviously no iso fields.
     rt::Descriptor::destructor =
       field_count > 0 ? VMObject::destructor_fn : nullptr;
-    rt::Descriptor::trace_possibly_iso =
-      field_count > 0 ? VMObject::trace_fn : nullptr;
+    // In the VM object we always need a finaliser in case it has an iso field.
+    // The finaliser will collect the iso references that will be deallocated
+    // at the end of this phase.
+    // TODO: Can be optimised if we look at the types of all the fields
     rt::Descriptor::finaliser =
-      finaliser_ip > 0 ? VMObject::finaliser_fn : nullptr;
+      finaliser_ip > 0 ? VMObject::finaliser_fn : VMObject::collect_iso_fields;
   }
 
   VMObject::VMObject(VMObject* region, const VMDescriptor* desc)
@@ -58,7 +60,7 @@ namespace verona::interpreter
     }
   }
 
-  void VMObject::trace_fn(const rt::Object* base_object, rt::ObjectStack* stack)
+  void VMObject::trace_fn(const rt::Object* base_object, rt::ObjectStack& stack)
   {
     const VMObject* object = static_cast<const VMObject*>(base_object);
     const VMDescriptor* descriptor = object->descriptor();
@@ -69,10 +71,30 @@ namespace verona::interpreter
     }
   }
 
-  void VMObject::finaliser_fn(rt::Object* base_object)
+  void VMObject::finaliser_fn(
+    rt::Object* base_object, rt::Object* region, rt::ObjectStack& sub_regions)
   {
     VMObject* object = static_cast<VMObject*>(base_object);
+
     VM::execute_finaliser(object);
+
+    collect_iso_fields(base_object, region, sub_regions);
+  }
+
+  void VMObject::collect_iso_fields(
+    rt::Object* base_object, rt::Object* region, rt::ObjectStack& sub_regions)
+  {
+    // The interpreter doesn't need the region, as the FieldValue type contains
+    // the ISO information in its fat pointers.
+    UNUSED(region);
+
+    VMObject* object = static_cast<VMObject*>(base_object);
+    const VMDescriptor* descriptor = object->descriptor();
+
+    for (size_t i = 0; i < descriptor->field_count; i++)
+    {
+      object->fields[i].add_isos(sub_regions);
+    }
   }
 
   void VMObject::destructor_fn(rt::Object* base_object)

src/interpreter/object.h

@@ -43,8 +43,15 @@ namespace verona::interpreter
 
     VMObject* region();
 
-    static void trace_fn(const rt::Object* base_object, rt::ObjectStack* stack);
-    static void finaliser_fn(rt::Object* base_object);
+    static void trace_fn(const rt::Object* base_object, rt::ObjectStack& stack);
+    static void finaliser_fn(
+      rt::Object* base_object,
+      rt::Object* region,
+      rt::ObjectStack& sub_regions);
+    static void collect_iso_fields(
+      rt::Object* base_object,
+      rt::Object* region,
+      rt::ObjectStack& sub_regions);
     static void destructor_fn(rt::Object* base_object);
 
   private:
@@ -81,16 +88,10 @@ namespace verona::interpreter
       rt::VCown<VMCown>::schedule();
     }
 
-    void trace(rt::ObjectStack* stack)
+    void trace(rt::ObjectStack& stack)
     {
       if (contents != nullptr)
-        stack->push(contents);
-    }
-
-    void trace_possibly_iso(rt::ObjectStack* stack)
-    {
-      if (contents != nullptr)
-        stack->push(contents);
+        stack.push(contents);
     }
   };
 }

src/interpreter/value.cc

@@ -297,18 +297,18 @@ namespace verona::interpreter
     return result;
   }
 
-  void FieldValue::trace(rt::ObjectStack* stack) const
+  void FieldValue::trace(rt::ObjectStack& stack) const
   {
     switch (tag)
     {
       case Value::ISO:
       case Value::MUT:
       case Value::IMM:
-        stack->push(inner.object);
+        stack.push(inner.object);
         break;
 
       case Value::COWN:
-        stack->push(inner.cown);
+        stack.push(inner.cown);
         break;
 
       case Value::UNINIT:
@@ -325,6 +325,31 @@ namespace verona::interpreter
     }
   }
 
+  void FieldValue::add_isos(rt::ObjectStack& stack) const
+  {
+    switch (tag)
+    {
+      case Value::ISO:
+        stack.push(inner.object);
+        break;
+
+      case Value::COWN:
+      case Value::MUT:
+      case Value::IMM:
+      case Value::UNINIT:
+      case Value::U64:
+      case Value::STRING:
+      case Value::DESCRIPTOR:
+        break;
+
+      case Value::COWN_UNOWNED:
+        // Cannot be part of the heap.
+        abort();
+
+        EXHAUSTIVE_SWITCH
+    }
+  }
+
   FieldValue::~FieldValue()
   {
     switch (tag)

src/interpreter/value.h

@@ -145,7 +145,7 @@ namespace verona::interpreter
       return &inner;
     }
 
-    void trace(rt::ObjectStack* stack) const;
+    void trace(rt::ObjectStack& stack) const;
 
     static constexpr Tag UNINIT = Tag::UNINIT;
     static constexpr Tag ISO = Tag::ISO;
@@ -190,7 +190,13 @@ namespace verona::interpreter
      */
     Value exchange(rt::Alloc* alloc, rt::Object* region, Value&& value);
 
-    void trace(rt::ObjectStack* stack) const;
+    void trace(rt::ObjectStack& stack) const;
+
+    /**
+     * If this Value contains an ISO, then add it to the stack
+     * Otherwise, do nothing.
+     **/
+    void add_isos(rt::ObjectStack& stack) const;
 
     friend fmt::formatter<Value>;
 

src/rt/cpp/vaction.h

@@ -27,7 +27,7 @@ namespace verona::rt
     friend class Cown;
 
   private:
-    static void gc_trace(const Action* msg, ObjectStack* st)
+    static void gc_trace(const Action* msg, ObjectStack& st)
     {
       (static_cast<const T*>(msg))->trace(st);
     }
@@ -51,7 +51,7 @@ namespace verona::rt
       return &desc;
     }
 
-    void trace(ObjectStack*) const {}
+    void trace(ObjectStack&) const {}
 
   public:
     VAction() : Action(desc())

src/rt/cpp/vobject.h

@@ -20,15 +20,6 @@ namespace verona::rt
   //
   // This is better than ignoring methods with the right name but the wrong
   // signature.
-  template<class T, class = void>
-  struct has_trace_possibly_iso : std::false_type
-  {};
-  template<class T>
-  struct has_trace_possibly_iso<
-    T,
-    std::void_t<decltype(&T::trace_possibly_iso)>> : std::true_type
-  {};
-
   template<class T, class = void>
   struct has_notified : std::false_type
   {};
@@ -66,27 +57,21 @@ namespace verona::rt
 
     using RegionClass = typename RegionType_to_class<region_type>::T;
 
-    static void gc_trace(const Object* o, ObjectStack* st)
+    static void gc_trace(const Object* o, ObjectStack& st)
     {
       ((T*)o)->trace(st);
     }
 
-    static void gc_trace_possibly_iso(const Object* o, ObjectStack* st)
-    {
-      if constexpr (has_trace_possibly_iso<T>::value)
-        ((T*)o)->trace_possibly_iso(st);
-    }
-
     static void gc_notified(Object* o)
     {
       if constexpr (has_notified<T>::value)
         ((T*)o)->notified(o);
     }
 
-    static void gc_final(Object* o)
+    static void gc_final(Object* o, Object* region, ObjectStack& sub_regions)
     {
       if constexpr (has_finaliser<T>::value)
-        ((T*)o)->finaliser();
+        ((T*)o)->finaliser(region, sub_regions);
     }
 
     static void gc_destructor(Object* o)
@@ -99,15 +84,14 @@ namespace verona::rt
       static constexpr Descriptor desc = {
         sizeof(T),
         gc_trace,
-        has_trace_possibly_iso<T>::value ? gc_trace_possibly_iso : nullptr,
         has_finaliser<T>::value ? gc_final : nullptr,
         has_notified<T>::value ? gc_notified : nullptr,
         has_destructor<T>::value ? gc_destructor : nullptr};
 
       return &desc;
     }
 
-    void trace(ObjectStack*) {}
+    void trace(ObjectStack&) {}
 
     static EpochMark get_alloc_epoch()
     {
@@ -131,7 +115,7 @@ namespace verona::rt
       if constexpr (std::is_same_v<Base, Object>)
         return RegionClass::template create<sizeof(T)>(alloc, desc());
       else
-        return ThreadAlloc::get()->alloc<sizeof(T)>();
+        return alloc->alloc<sizeof(T)>();
     }
 
     void* operator new(size_t, Object* region)
@@ -148,7 +132,7 @@ namespace verona::rt
       if constexpr (std::is_same_v<Base, Object>)
         return RegionClass::template alloc<sizeof(T)>(alloc, region, desc());
       else
-        return ThreadAlloc::get()->alloc<sizeof(T)>();
+        return alloc->alloc<sizeof(T)>();
     }
 
     void operator delete(void*)

src/rt/object/object.h

@@ -85,8 +85,7 @@ namespace verona::rt
   {
     // for field in o do
     //  st.push(o.field)
-    using TraceFunction = void (*)(const Object* o, ObjectStack* st);
-    using TraceIsoFunction = void (*)(const Object* o, ObjectStack* st);
+    using TraceFunction = void (*)(const Object* o, ObjectStack& st);
 
     using NotifiedFunction = void (*)(Object* o);
 
@@ -102,14 +101,15 @@ namespace verona::rt
     // invalid state ie. close file descriptors or deallocate some auxiliary
     // storage.
     //
-    // The finaliser can extract sub-regions and eg. send them in a
-    // multi-message. The destructor on the other hand may not do this.
-    using FinalFunction = void (*)(Object* o);
+    // The finaliser can must add all the subregions reachable from this object
+    // to the ObjectStack it is passed, so that they can be deallocated once
+    // this region has been deallocated.
+    using FinalFunction = void (*)(Object* o, Object* region, ObjectStack& st);
+
     using DestructorFunction = void (*)(Object* o);
 
     size_t size;
     TraceFunction trace;
-    TraceIsoFunction trace_possibly_iso;
     FinalFunction finaliser;
     NotifiedFunction notified = nullptr;
     DestructorFunction destructor = nullptr;
@@ -702,15 +702,9 @@ namespace verona::rt
       return get_descriptor()->destructor != nullptr;
     }
 
-    inline bool has_possibly_iso_fields()
-    {
-      return get_descriptor()->trace_possibly_iso != nullptr;
-    }
-
     static inline bool is_trivial(const Descriptor* desc)
     {
-      return desc->destructor == nullptr && desc->finaliser == nullptr &&
-        desc->trace_possibly_iso == nullptr;
+      return desc->destructor == nullptr && desc->finaliser == nullptr;
     }
 
     inline bool is_trivial()
@@ -729,18 +723,13 @@ namespace verona::rt
   private:
     inline void trace(ObjectStack& f) const
     {
-      get_descriptor()->trace(this, &f);
-    }
-
-    inline void trace_possibly_iso(ObjectStack& f) const
-    {
-      get_descriptor()->trace_possibly_iso(this, &f);
+      get_descriptor()->trace(this, f);
     }
 
-    inline void finalise()
+    inline void finalise(Object* region, ObjectStack& isos)
     {
       if (has_finaliser())
-        get_descriptor()->finaliser(this);
+        get_descriptor()->finaliser(this, region, isos);
     }
 
     inline void notified()
@@ -760,21 +749,23 @@ namespace verona::rt
       alloc->dealloc(this, size());
     }
 
-    inline void
-    find_iso_fields(Object* region_entry, ObjectStack& f, ObjectStack& iso_st)
+  protected:
+    static void
+    add_sub_region(Object* obj, Object* region, ObjectStack& sub_regions)
     {
-      // Find all isolated fields in p other than some predecessor o.
-      if (!has_possibly_iso_fields())
-        return;
-
-      trace_possibly_iso(f);
-
-      while (!f.empty())
+      // Should be the entry-point of the region.
+      assert(
+        (region == nullptr) || (region->get_class() == Object::RegionMD::ISO));
+      // Have to be careful about internal references to the entry point for the
+      // `region` i.e. when obj == region we are refering to the entry point
+      // from inside the region and should not treat this as a subregion
+      // pointer.
+      if ((obj != nullptr) && (obj != region))
       {
-        Object* o = f.pop();
-
-        if ((o != region_entry) && (o->get_class() == Object::ISO))
-          iso_st.push(o);
+        if (obj->get_class() == Object::RegionMD::ISO)
+        {
+          sub_regions.push(obj);
+        }
       }
     }
   };

src/rt/region/externalreference.h

@@ -33,7 +33,7 @@ namespace verona::rt
       // The object externally referred to
       Object* o;
 
-      static void gc_trace(const Object*, ObjectStack*) {}
+      static void gc_trace(const Object*, ObjectStack&) {}
 
       static const Descriptor* desc()
       {