Merge pull request #6648 from jdmpapin/reg-dtor

Replace TR::Region::create() with registerDestructor()

compiler/env/Region.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2016 IBM Corp. and others
+ * Copyright (c) 2000, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -33,7 +33,7 @@ Region::Region(TR::SegmentProvider &segmentProvider, TR::RawAllocator rawAllocat
    _rawAllocator(rawAllocator),
    _initialSegment(_initialSegmentArea.data, INITIAL_SEGMENT_SIZE),
    _currentSegment(TR::ref(_initialSegment)),
-   _lastDestructable(NULL)
+   _lastDestroyer(NULL)
    {
    }
 
@@ -43,7 +43,7 @@ Region::Region(const Region &prototype) :
    _rawAllocator(prototype._rawAllocator),
    _initialSegment(_initialSegmentArea.data, INITIAL_SEGMENT_SIZE),
    _currentSegment(TR::ref(_initialSegment)),
-   _lastDestructable(NULL)
+   _lastDestroyer(NULL)
    {
    }
 
@@ -53,12 +53,11 @@ Region::~Region() throw()
     * Destroy all object instances that depend on the region
     * to manage their lifetimes.
     */
-   Destructable *lastDestructable = _lastDestructable;
-   while (lastDestructable)
+   Destroyer *lastDestroyer = _lastDestroyer;
+   while (lastDestroyer != NULL)
       {
-      Destructable * const currentDestructable = lastDestructable;
-      lastDestructable = currentDestructable->prev();
-      currentDestructable->~Destructable();
+      lastDestroyer->destroy();
+      lastDestroyer = lastDestroyer->prev();
       }
 
    for (

compiler/env/Region.hpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2017 IBM Corp. and others
+ * Copyright (c) 2000, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -40,48 +40,30 @@ class RegionProfiler;
 class Region
    {
 
-   /**
-    * @brief The Destructable class provides a common base class, providing an
-    * interface or invoking the destructor of allocated objects of arbitrary type.
-    */
-   class Destructable
+   /** \brief A list entry that knows an object to destroy, and how to destroy it. */
+   class Destroyer
       {
    public:
-      Destructable(Destructable * const prev) :
-         _prev(prev)
-         {
-         }
-
-      Destructable * prev() { return _prev; }
-
-      virtual ~Destructable() throw() {}
+      Destroyer(Destroyer *prev) : _prev(prev) {}
+      Destroyer *prev() { return _prev; }
+      virtual void destroy() = 0;
 
    private:
-      Destructable * const _prev;
+      Destroyer * const _prev;
       };
 
    /**
-    * @brief A class template allowing the creation of allocation instances of arbitrary
-    * type, all of which have a predictable means of accessing their destructor.
+    * \brief A Destroyer that destroys an object of type \p T.
     */
    template <typename T>
-   class Instance : public Destructable
+   class TypedDestroyer : public Destroyer
       {
    public:
-      Instance(Destructable *prev, const T& value) :
-         Destructable(prev),
-         _instance(value)
-         {
-         }
-
-      ~Instance() throw()
-         {
-         }
-
-      T& value() { return _instance; }
+      TypedDestroyer(Destroyer *prev, T *obj) : Destroyer(prev), _obj(obj) {}
+      virtual void destroy() { _obj->~T(); }
 
    private:
-      T _instance;
+      T * const _obj;
       };
 
 public:
@@ -91,25 +73,69 @@ class Region
    void * allocate(const size_t bytes, void * hint = 0);
 
    /**
-    * @brief A function template to create a Region-managed object instance.
+    * \brief Register an object to be explicitly destroyed along with this Region.
+    *
+    * <em>Because Region frees memory in bulk, such registration is
+    * unnecessary for almost all objects.</em>
+    *
+    * All registered objects will be destroyed in LIFO order when this
+    * Region is destroyed, just prior to freeing the backing memory. As such:
+    *
+    * (a) \p obj must have been allocated by this Region, and
+    * (b) objects should generally be registered in construction order.
+    *
+    * To ensure that these hold, register immediately after new like so:
+    *
+      \verbatim
+         TheOneRing *ring = new (region) TheOneRing(...); // must be destroyed
+         region.registerDestructor(ring);
+      \endverbatim
+    *
+    * A previous version of this API attempted to guarantee (a) and (b) by
+    * fusing this registration with the allocation and construction of the
+    * object to be registered, but in so doing, it required the object's type
+    * to be copy-constructible. It's possible to fuse without that requirement
+    * (and without requiring it to be default-constructible), but only with
+    * variadic macros, which are not necessarily supported on all build
+    * compilers.
     *
-    * @param[in] prototype A copy-constructible instance used to initialize the Region-managed instance.
+    * <em>Pointer type:</em> The type \p T at the call site is the type whose
+    * destructor will be called to destroy \p obj. Therefore, \p T must be
+    * either the exact concrete type of \p obj or a base type with a virtual
+    * destructor. If \p T is a base type without a virtual destructor, then
+    * \p obj will not be properly destroyed.
+    *
+    * <em>Possibly unintuitive relative lifetime of registered objects:</em>
+    * If a Region R0 is used to create and register another Region R1 (or an
+    * object embedding another Region R1 by value), then any objects created
+    * and registered in R1 will outlive objects registered in R0 after the
+    * registration of R1. This may be surprising because all \em unregistered
+    * objects in R0 outlive all \em unregistered objects in R1. The following
+    * example shows the order of destruction of the (non-Region) objects:
+    *
+      \verbatim
+         Foo *foo = new (R0) Foo(...);
+         R0.registerDestructor(foo);      // third destroyed
+
+         TR::Region *R1 = new (R0) TR::Region(...);
+         R0.registerDestructor(R1);
+
+         Bar *bar = new (R0) Bar(...);
+         R0.registerDestructor(bar);      // first destroyed
+
+         Baz *baz = new (*R1) Baz(...);
+         R1->registerDestructor(baz);      // second destroyed (while destroying R1)
+      \endverbatim
+    *
+    * \param[in] obj The object that should eventually be destroyed. It must
+    *                have been allocated by this Region.
     *
-    * Object instances created in this manner will be destroyed in LIFO order when the owning
-    * Region is destroyed.\n
-    * NOTE: If, using a region R0, a second Region R1 is instantiated (directly or indirectly)
-    * via this mechanism, any objects created in R0 after the instantiation of R1 will have a
-    * shorter life span than all the objects created in R1.\n
-    * \n
-    * Given the context of R0 and R1 above:\n
-    * Objects created in R0 before R1 > Objects created in R1 > Objects created in R0 after R1\n
-    * \n
-    * This is most likely going to be invoked using a temporary:\n
-    * ```region.create( MyClass(arg0, ...argN) );```\n
-    * Or, if using the default constructor, use extra parentheses to avoid the 'most vexing parse':\n
-    * ```region.create( (MyClass()) );```
     */
-   template <typename T> inline T& create(const T& prototype);
+   template <typename T>
+   void registerDestructor(T *obj)
+      {
+      _lastDestroyer = new (*this) TypedDestroyer<T>(_lastDestroyer, obj);
+      }
 
    void deallocate(void * allocation, size_t = 0) throw();
 
@@ -148,7 +174,7 @@ class Region
    TR::MemorySegment _initialSegment;
    TR::reference_wrapper<TR::MemorySegment> _currentSegment;
 
-   Destructable *_lastDestructable;
+   Destroyer *_lastDestroyer;
 
    static const size_t INITIAL_SEGMENT_SIZE = 4096;
 
@@ -166,13 +192,4 @@ inline void operator delete(void * p, TR::Region &region) { region.deallocate(p)
 inline void * operator new[](size_t size, TR::Region &region) { return region.allocate(size); }
 inline void operator delete[](void * p, TR::Region &region) { region.deallocate(p); }
 
-template <typename T>
-T &
-TR::Region::create(const T &value)
-   {
-   Instance<T> *instance = new (*this) Instance<T>(_lastDestructable, value);
-   _lastDestructable = static_cast<Destructable *>(instance);
-   return instance->value();
-   }
-
 #endif // OMR_REGION_HPP