changeset 29943:513c4fd440b1

simplify implementation of dim_vector class Instead of a special style of reference counting (count and number of dimensions both stored in the data array) just use a simple allocated array and define a move constructor and move assignment operator. * dim-vector.h, dim-vector.cc (dim_vector::m_ndims): New data member. Replace all uses of m_rep[-1] with m_ndims. Update constructors to define m_ndims. (dim_vector::chop_trailing_singletons, dim_vector::resize): Simplify. (dim_vector::nil_rep, dim_vector::newrep): Delete static methods. (dim_vector::count, dim_vector::increment_count, dim_vector::decrement_count, dim_vector::clonerep, dim_vector::resizerep, dim_vector::freerep, dim_vector::make_unique): Delete methods and all uses. * jit-typeinfo.h, jit-typeinfo.cc (jit_array::m_ndims): New data member. (jit_array::update): Set m_ndims insetead of accessing from m_dimensions. (octave_jit_end_matrix): Use mat->m_ndims instead of accessing mat->m_dimensions[-1].
author John W. Eaton <jwe@octave.org>
date Wed, 04 Aug 2021 00:39:27 -0400
parents 7dbbdd00db4a
children 029880dbbebb
files libinterp/parse-tree/jit-typeinfo.cc libinterp/parse-tree/jit-typeinfo.h liboctave/array/dim-vector.cc liboctave/array/dim-vector.h
diffstat 4 files changed, 65 insertions(+), 138 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/parse-tree/jit-typeinfo.cc	Wed Aug 04 21:32:53 2021 -0700
+++ b/libinterp/parse-tree/jit-typeinfo.cc	Wed Aug 04 00:39:27 2021 -0400
@@ -346,7 +346,7 @@
   octave_jit_end_matrix (jit_matrix *mat, octave_idx_type idx,
                          octave_idx_type count)
   {
-    octave_idx_type ndim = mat->m_dimensions[-1];
+    octave_idx_type ndim = mat->m_ndims;
     if (ndim == count)
       return mat->m_dimensions[idx];
     else if (ndim > count)
--- a/libinterp/parse-tree/jit-typeinfo.h	Wed Aug 04 21:32:53 2021 -0700
+++ b/libinterp/parse-tree/jit-typeinfo.h	Wed Aug 04 00:39:27 2021 -0400
@@ -89,6 +89,7 @@
       m_ref_count = m_array->jit_ref_count ();
       m_slice_data = m_array->jit_slice_data () - 1;
       m_slice_len = m_array->numel ();
+      m_ndims = m_array->ndims ();
       m_dimensions = m_array->jit_dimensions ();
     }
 
@@ -107,6 +108,7 @@
 
     U *m_slice_data;
     octave_idx_type m_slice_len;
+    octave_idx_type m_ndims;
     octave_idx_type *m_dimensions;
 
     T *m_array;
--- a/liboctave/array/dim-vector.cc	Wed Aug 04 21:32:53 2021 -0700
+++ b/liboctave/array/dim-vector.cc	Wed Aug 04 00:39:27 2021 -0400
@@ -36,21 +36,6 @@
 #include "Array.h"
 #include "dim-vector.h"
 
-octave_idx_type *
-dim_vector::nil_rep (void)
-{
-  // Create a statically allocated rep object with an initial reference
-  // count of 1.  The dim_vector constructor that uses this object will
-  // increment the reference count.  The dim_vector destructor and copy
-  // assignment operator will decrement the reference count but those
-  // operations can never cause the count to become zero so they will
-  // never call delete on this object.
-
-  static octave_idx_type nr[4] = { 1, 2, 0, 0 };
-
-  return &nr[2];
-}
-
 // The maximum allowed value for a dimension extent.  This will normally be a
 // tiny bit off the maximum value of octave_idx_type.
 // Currently 1 is subtracted to allow safe conversion of any 2D Array into
@@ -64,8 +49,6 @@
 void
 dim_vector::chop_all_singletons (void)
 {
-  make_unique ();
-
   int j = 0;
   int nd = ndims ();
 
@@ -78,7 +61,7 @@
   if (j == 1)
     m_rep[1] = 1;
 
-  m_rep[-1] = (j > 2 ? j : 2);
+  m_ndims = (j > 2 ? j : 2);
 }
 
 std::string
@@ -164,8 +147,6 @@
   else
     new_nd = orig_nd;
 
-  make_unique ();
-
   bool match = true;
 
   for (int i = 0; i < ndb; i++)
--- a/liboctave/array/dim-vector.h	Wed Aug 04 21:32:53 2021 -0700
+++ b/liboctave/array/dim-vector.h	Wed Aug 04 00:39:27 2021 -0400
@@ -95,85 +95,9 @@
 {
 private:
 
-  octave_idx_type *m_rep;
-
-  octave_idx_type& count (void) const { return m_rep[-2]; }
-
-  octave_idx_type increment_count (void)
-  {
-    return octave_atomic_increment (&(count ()));
-  }
-
-  octave_idx_type decrement_count (void)
-  {
-    return octave_atomic_decrement (&(count ()));
-  }
-
-  //! Construct a new rep with count = 1 and ndims given.
-
-  static octave_idx_type * newrep (int ndims)
-  {
-    octave_idx_type *r = new octave_idx_type [ndims + 2];
-
-    *r++ = 1;
-    *r++ = ndims;
-
-    return r;
-  }
-
-  //! Clone this->m_rep.
-
-  octave_idx_type * clonerep (void)
-  {
-    int nd = ndims ();
-
-    octave_idx_type *r = newrep (nd);
-
-    std::copy_n (m_rep, nd, r);
-
-    return r;
-  }
+  octave_idx_type m_ndims;
 
-  //! Clone and resize this->m_rep to length n, filling by given value.
-
-  octave_idx_type * resizerep (int n, octave_idx_type fill_value)
-  {
-    int nd = ndims ();
-
-    if (n < 2)
-      n = 2;
-
-    octave_idx_type *r = newrep (n);
-
-    if (nd > n)
-      nd = n;
-
-    std::copy_n (m_rep, nd, r);
-    std::fill_n (r + nd, n - nd, fill_value);
-
-    return r;
-  }
-
-  //! Free the rep.
-
-  void freerep (void)
-  {
-    assert (count () == 0);
-    delete [] (m_rep - 2);
-  }
-
-  void make_unique (void)
-  {
-    if (count () > 1)
-      {
-        octave_idx_type *new_rep = clonerep ();
-
-        if (decrement_count () == 0)
-          freerep ();
-
-        m_rep = new_rep;
-      }
-  }
+  octave_idx_type *m_rep;
 
 public:
 
@@ -214,12 +138,13 @@
 
   template <typename... Ints>
   dim_vector (const octave_idx_type r, const octave_idx_type c,
-              Ints... lengths) : m_rep (newrep (2 + sizeof... (Ints)))
+              Ints... lengths)
+    : m_ndims (2 + sizeof... (Ints)), m_rep (new octave_idx_type [m_ndims])
   {
     std::initializer_list<octave_idx_type> all_lengths = {r, c, lengths...};
+    octave_idx_type *ptr = m_rep;
     for (const octave_idx_type l: all_lengths)
-      *m_rep++ = l;
-    m_rep -= all_lengths.size ();
+      *ptr++ = l;
   }
 
   // Fast access with absolutely no checking
@@ -232,7 +157,6 @@
 
   octave_idx_type& elem (int i)
   {
-    make_unique ();
     return xelem (i);
   }
 
@@ -240,15 +164,8 @@
 
   void chop_trailing_singletons (void)
   {
-    int nd = ndims ();
-    if (nd > 2 && m_rep[nd-1] == 1)
-      {
-        make_unique ();
-        do
-          nd--;
-        while (nd > 2 && m_rep[nd-1] == 1);
-        m_rep[-1] = nd;
-      }
+    while (m_ndims > 2 && m_rep[m_ndims-1] == 1)
+      m_ndims--;
   }
 
   OCTAVE_API void chop_all_singletons (void);
@@ -261,37 +178,52 @@
 
 private:
 
-  static octave_idx_type *nil_rep (void);
+  explicit dim_vector (octave_idx_type ndims)
+    : m_ndims (ndims < 2 ? 2 : ndims), m_rep (new octave_idx_type [m_ndims])
+  {
+    std::fill_n (m_rep, m_ndims, 0);
+  }
 
 public:
 
   static OCTAVE_API octave_idx_type dim_max (void);
 
-  explicit dim_vector (void) : m_rep (nil_rep ())
-  { increment_count (); }
+  explicit dim_vector (void)
+    : m_ndims (2), m_rep (new octave_idx_type [m_ndims])
+  {
+    std::fill_n (m_rep, m_ndims, 0);
+  }
 
-  dim_vector (const dim_vector& dv) : m_rep (dv.m_rep)
-  { increment_count (); }
+  dim_vector (const dim_vector& dv)
+    : m_ndims (dv.m_ndims), m_rep (new octave_idx_type [m_ndims])
+  {
+    std::copy_n (dv.m_rep, m_ndims, m_rep);
+  }
 
-  dim_vector (dim_vector&& dv) : m_rep (dv.m_rep) { dv.m_rep = nullptr; }
+  // FIXME: Should be private, but required by array constructor for jit
+  explicit dim_vector (octave_idx_type *r) : m_rep (r) { }
 
-// FIXME: Should be private, but required by array constructor for jit
-  explicit dim_vector (octave_idx_type *r) : m_rep (r) { }
+  dim_vector (dim_vector&& dv)
+    : m_ndims (0), m_rep (nullptr)
+  {
+    *this = std::move (dv);
+  }
 
   static dim_vector alloc (int n)
   {
-    return dim_vector (newrep (n < 2 ? 2 : n));
+    return dim_vector (n);
   }
 
   dim_vector& operator = (const dim_vector& dv)
   {
     if (&dv != this)
       {
-        if (decrement_count () == 0)
-          freerep ();
+        delete [] m_rep;
 
-        m_rep = dv.m_rep;
-        increment_count ();
+        m_ndims = dv.m_ndims;
+        m_rep = new octave_idx_type [m_ndims];
+
+        std::copy_n (dv.m_rep, m_ndims, m_rep);
       }
 
     return *this;
@@ -305,10 +237,12 @@
         // operator, m_rep may be a nullptr here.  We should only need to
         // protect the destructor in a similar way.
 
-        if (m_rep && decrement_count () == 0)
-          freerep ();
+        delete [] m_rep;
 
+        m_ndims = dv.m_ndims;
         m_rep = dv.m_rep;
+
+        dv.m_ndims = 0;
         dv.m_rep = nullptr;
       }
 
@@ -321,8 +255,7 @@
     // operator, m_rep may be a nullptr here.  We should only need to
     // protect the move assignment operator in a similar way.
 
-    if (m_rep && decrement_count () == 0)
-      freerep ();
+    delete [] m_rep;
   }
 
   //! Number of dimensions.
@@ -331,7 +264,7 @@
   //! elements in the dim_vector including trailing singletons.  It is also
   //! the number of dimensions an Array with this dim_vector would have.
 
-  octave_idx_type ndims (void) const { return m_rep[-1]; }
+  octave_idx_type ndims (void) const { return m_ndims; }
 
   //! Number of dimensions.
   //! Synonymous with ndims().
@@ -348,17 +281,28 @@
 
   void resize (int n, int fill_value = 0)
   {
-    int len = ndims ();
+    if (n < 2)
+      n = 2;
 
-    if (n != len)
-      {
-        octave_idx_type *r = resizerep (n, fill_value);
+    if (n == m_ndims)
+      return;
 
-        if (decrement_count () == 0)
-          freerep ();
+    if (n < m_ndims)
+      {
+        m_ndims = n;
+        return;
+      }
+
+    octave_idx_type *new_rep = new octave_idx_type [n];
 
-        m_rep = r;
-      }
+    std::copy_n (m_rep, m_ndims, new_rep);
+    std::fill_n (new_rep + m_ndims, n - m_ndims, fill_value);
+
+    delete [] m_rep;
+
+    m_rep = new_rep;
+
+    m_ndims = n;
   }
 
   OCTAVE_API std::string str (char sep = 'x') const;