Re: Switch to std::atomic?

[John W. Eaton]

On 9/25/19 10:34 AM, John W. Eaton wrote:
> On 9/13/19 5:29 PM, Rik wrote:
>
> > Would it make sense to switch to using std::atomic<octave_idx_type>?  In
> > that case, Octave would be portable to any machine with a valid C++ 
> > runtime
> > library, and we certainly require that.  The std::atomic class overloads
> > the (pre-, post-)(increment, decrement) operators so one could write more
> > readable code with count++ rather than 
> > OCTAVE_ATOMIC_POST_INCREMENT(count).
>
> Yes, now that it is a standard C++ feature, we should be using that. For 
> a simple transition, can we first define our refcount class using 
> std::atomic<T>?
>
> Would you like to make this change?  If not, I can look at it.

I was curious to see what it would take and came up with the following 
change.  I think this is OK as far as it goes, but I haven't pushed it.

Notes and questions:

  * The C11 atomic_fetch_add and atomic_fetch_sub functions were 
required to handle the tricky way that the dim_vector class stores the 
reference count.  I believe that to do this job in C++ requires the 
atomic_ref object that is a new feature in C++20.  Either that or use a 
separate reference count variable instead of storing it in the array of 
values, but then as the comment in dim-vector.h says, we will need two 
allocations, one for the rep object and one for the array that it will 
then contain.

  * Some reference counts use int and some use octave_idx_type.  Maybe 
we should always use the same type?  Should it be int or octave_idx_type 
or size_t?

  * The copy constructor for the cdef_class_rep object looks wrong to 
me.  Instead of copying the reference count, shouldn't the count for the 
new object be 1?  But I'm not sure I understand the way this code is 
supposed to work so I left it alone and provided a copy constructor for 
the refcount class.

jwe

refcount-diffs.txt
Description: Text document