Few Atomic class improvement


#1
// generally:
template <typename T> struct AllowedBasicType 
{
     // If compiler breaks here, it's because:
     // - You should avoid using Atomic with non-POD type
     char array[sizeof(T) - 16];
     // - You  should use a type with size being a multiple of supported register's size.
     char array[1 - (sizeof(T) % sizeof(__LONGTYPE__))];
};  
template <> AllowedBasicType<int> { };
template <> AllowedBasicType<float> { };
template <> AllowedBasicType<double> { };
template <> AllowedBasicType<unsigned int> { };
template <> AllowedBasicType<void *> { };

// In Atomic class:
template <typename Type>
class Atomic : public AllowedBasicType<Type>
{
/** Atomically increments this value, returning the previous value. */
Type operator++(int) throw();

/** Atomically decrements this value, returning the previous value. */
Type operator--(int) throw();

#2

I deliberately didn’t include the post-increment operators in the class, because their behaviour is ambiguous - does it store the previous value of the variable, increment it, and return the old one (problematic because the value may be changed in between these operations); or does it increment the variable and return the result - 1 (problematic because the value returned may never have actually been held by the variable)… Best to avoid this and let the user implement the behaviour that they need using other operators.

Is that code portable? I’ve never heard of that LONGTYPE token before - is it available on all compilers?? And I thought vc6 didn’t support template specialisation?.. Plus, won’t this bloat the object with useless base class array storage if you create an Atomic ?


#3

I don’t know how portable this is so yes, you’re right, it would be a nightmare to debug if the behaviour is not the same on all platform.

LONGTYPE is not a portable macro. I think it’s perfectly correct to use sizeof(int) instead.

vs6 doesn’t support partial template specialization (that is “template struct AllowedBasicType<T *> {…};”). But it supports full template specialization (hopefully, template would have been very limited without).

I’m not 100% sure about this, but I’m sure that "sizeof(emptyClassWithEmptyParent) == sizeof(emptyClass) == 1"
In my compilers (vc6, gcc 4.3.0, icc 11), it doesn’t change sizeof(Atomic) with or without base class.
But since standard says that including a virtual table when inheriting is possible, it’s not 100% safe that it won’t bloat.
Maybe it’s a good idea to add an AllowedBasicType as a member in a “#if JUCE_DEBUG” section ? Doing so, will still add at least 1 bytes to the class anyway (hence the inheritance).

The goal of this code is to prevent code like Atomic or Atomic from compiling, instead of crashing at first compareAndSwap because it deals with 4 bytes.
It doesn’t compile in that case because when instantiating the template “AllowedBasicType”, the compiler must create an negative-sized array which is illegal.


#4

Also, you can put a jstatic_assert instead, but I don’t know how they are implemented and might still bloat here anyway.


#5

I already did that - there’s a static assertion in the Atomic destructor that prevents those from compiling… doesn’t it work for you?


#6

I haven’t seen it. It should work. Just a thought, through, on a 32bits machine, is it mandatory that 64 bits operations are atomic ?
Anyway, my atomic class used to use that trick, but the test in the destructor will work also (and it’s more simple and more elegant).
Since I’m excaving my code, I’ve made a SharedDataWriter class that use a reference instead of the value in the atomic class. (Type & value; instead of Type value;)
The idea is if you can’t modify an existing class header, but still want to write atomically to one of the Type in there, you’ll write code like this:

{
    SharedDataWriter sdw(a.member);
    sdw = something;
}

Handy class avoiding to pay the lock price.


#7

Interesting idea about the SharedDataWriter, though it’s probably not good to allow a variable to seem atomic while allowing non-atomic access to it too.

There was one place in the code where I needed to do the same thing, and where I couldn’t use an Atomic object, but I think I just did a reinterpret_cast of a void* to an Atomic<void*> and called the method that I needed. A bit hacky, but it works just fine.