Why having a destructor sometimes could be a bad thing

It looks like ArrayAllocationBase.h has been recently replaced by ArrayBase.h, exhibiting some long awaited (at least by me!) performance boosts.

If you have some simple struct (or a class) like this e.g.

struct MyStruct
{
   MyStruct(int sample_, float value_)
	: sample(sample_)
	, value(value_)
	{}

	int sample = 1;
	float value = 2.0f;
};

and some arrays like Array<MyStruct> array1, array2;

and want to add them with

array1.addArray(array2)

the copy handling code in Array will now eventually boil down to

memcpy(elements + numUsed, otherElements, (size_t)numElements * sizeof(ElementType));

instead of previously a loop like

	auto* start = elements + numUsed;

	while (--numElements >= 0)
		new (start++) ElementType(*(otherElements++));

which certainly looks much slower when you’re adding, say some 10 000 automation events…
And it’s not just appending one array to another that’s speedier now. Also creating, inserts and other bulky operations will gain from the face lifted Array class (and OwnedArray).

But what about the destructor?
-Well, which copy method to use (memcpy or loop) is determined by checking

std::is_trivially_copyable<MyStruct>::value; 

which evaluates to true if the class/struct is trivially copyable (and false if not).

And for a class to be trivially copyable it mustn’t have a destructor (there’s a few other criterias as well, google!) which means if you add even an empty destructor like

~MyStruct()
{
}

you’re stuck with the old tedious element by element copying. (Which by the way happened to me, I had an empty destructor for debugging purposes, and had forgotten to remove it or just optimistically thought it would be removed by the optimizer…)

So, if you’re a hard core Array user (and haven’t yet switched to std::vector<>) you might want to make the objects you stuff in the Array trivially copyable for some performance boost!

5 Likes

Yep, “Rule of Zero”: https://en.cppreference.com/w/cpp/language/rule_of_three

1 Like

Are you sure you’ve seen a performance boost? The ArrayAllocationBase class also used basic memory copies to move elements around. One of the motivations for introducing ArrayBase was to allow storage of non-TriviallyCopyable types in JUCE’s containers. Before these changes we always assumed TriviallyCopyable types (with just a warning in the docs) so you could end up with some really strange run-time errors if you unknowingly did something like Array<std::function<void()>>.

My advice is to put = default after every constructor you possibly can. There are a few commits after the Array changes where I worked my way through the codebase doing this to make as many classes as possible TriviallyCopyable. This will give you better performance in all sorts of other places in addition to JUCE’s containers.

For TriviallyCopyable types JUCE’s Array outperforms std::vector if your array sizes are dynamic. We’re free to use realloc behind the scenes which means we can often avoid copying all the elements in the array when the storage capacity increases. This is not something that’s possible with std::vector, even with a custom allocator.

2 Likes

The old Array implementation used loop copying when doing addArray() and when creating new Arrays from initializer list. The only use of bulk copying afaik was to make space for the new elements when inserting etc.

I went with my C++ intuition that memcpy is faster than “manual” copying element by element above some arbitrary array size. I even pathched Array.h with an addArrayFast() implementation that used memcpy to speed things up a bit. Something I now happily can remove thanks to your updated Array/Arraybase implementation.

If you hadn’t said otherwise, from only comparing the old code with the new I would have thought the new implementation was to speed up some array methods (by using memcpy) for trivially copyable objects, while maintaining the old performance for non trivially copyable. But, I’m no expert…

Nice to hear that Juce Array outperforms std::vector in some cases, no use in feeling retarded just because you’re not switching to std::vector :slight_smile:

My advice is to put = default after every constructor you possibly can.

Hmm, you mean after every default constructor like MyClass() {}? Because then they’d be trivial and possibly trivially copyable?

Yep.