From 244f46582b8f6ac158aa46e106256ba726cc1819 Mon Sep 17 00:00:00 2001 From: falkTX Date: Wed, 29 Nov 2017 22:02:32 +0100 Subject: [PATCH] Cleanup, noexcept and other small changes in water module --- .../modules/water/buffers/AudioSampleBuffer.h | 137 ++++-------------- source/modules/water/containers/Array.h | 57 +++++--- .../water/containers/ArrayAllocationBase.h | 26 ++-- source/modules/water/containers/OwnedArray.h | 22 +-- source/modules/water/memory/HeapBlock.h | 18 +-- source/modules/water/misc/Time.cpp | 13 +- 6 files changed, 97 insertions(+), 176 deletions(-) diff --git a/source/modules/water/buffers/AudioSampleBuffer.h b/source/modules/water/buffers/AudioSampleBuffer.h index ff470d503..1a3b34294 100644 --- a/source/modules/water/buffers/AudioSampleBuffer.h +++ b/source/modules/water/buffers/AudioSampleBuffer.h @@ -198,7 +198,7 @@ public: size (other.size), allocatedBytes (other.allocatedBytes), channels (other.channels), - allocatedData (static_cast&&> (other.allocatedData)), + allocatedData (static_cast&&> (other.allocatedData)), isClear (other.isClear) { memcpy (preallocatedChannelSpace, other.preallocatedChannelSpace, sizeof (preallocatedChannelSpace)); @@ -214,7 +214,7 @@ public: size = other.size; allocatedBytes = other.allocatedBytes; channels = other.channels; - allocatedData = static_cast&&> (other.allocatedData); + allocatedData = static_cast&&> (other.allocatedData); isClear = other.isClear; memcpy (preallocatedChannelSpace, other.preallocatedChannelSpace, sizeof (preallocatedChannelSpace)); other.numChannels = 0; @@ -323,14 +323,14 @@ public: If the required memory can't be allocated, this will throw a std::bad_alloc exception. */ - void setSize (int newNumChannels, + bool setSize (int newNumChannels, int newNumSamples, bool keepExistingContent = false, bool clearExtraSpace = false, bool avoidReallocating = false) noexcept { - jassert (newNumChannels >= 0); - jassert (newNumSamples >= 0); + CARLA_SAFE_ASSERT_RETURN (newNumChannels >= 0, false); + CARLA_SAFE_ASSERT_RETURN (newNumSamples >= 0, false); if (newNumSamples != size || newNumChannels != numChannels) { @@ -341,7 +341,7 @@ public: if (keepExistingContent) { - HeapBlock newData; + HeapBlock newData; newData.allocate (newTotalBytes, clearExtraSpace || isClear); const size_t numSamplesToCopy = (size_t) jmin (newNumSamples, size); @@ -375,8 +375,8 @@ public: } else { + CARLA_SAFE_ASSERT_RETURN (allocatedData.allocate (newTotalBytes, clearExtraSpace || isClear), false); allocatedBytes = newTotalBytes; - allocatedData.allocate (newTotalBytes, clearExtraSpace || isClear); channels = reinterpret_cast (allocatedData.getData()); } @@ -392,6 +392,8 @@ public: size = newNumSamples; numChannels = newNumChannels; } + + return true; } /** Makes this buffer point to a pre-allocated set of channel data arrays. @@ -412,12 +414,12 @@ public: @param newNumSamples the number of samples to use - this must correspond to the size of the arrays passed in */ - void setDataToReferTo (float** dataToReferTo, + bool setDataToReferTo (float** dataToReferTo, const int newNumChannels, const int newNumSamples) noexcept { - jassert (dataToReferTo != nullptr); - jassert (newNumChannels >= 0 && newNumSamples >= 0); + CARLA_SAFE_ASSERT_RETURN (dataToReferTo != nullptr, false); + CARLA_SAFE_ASSERT_RETURN (newNumChannels >= 0 && newNumSamples >= 0, false); if (allocatedBytes != 0) { @@ -428,8 +430,7 @@ public: numChannels = newNumChannels; size = newNumSamples; - allocateChannels (dataToReferTo, 0); - jassert (! isClear); + return allocateChannels (dataToReferTo, 0); } /** Resizes this buffer to match the given one, and copies all of its content across. @@ -948,112 +949,22 @@ public: } } -#if 0 - /** Returns a Range indicating the lowest and highest sample values in a given section. - - @param channel the channel to read from - @param startSample the start sample within the channel - @param numSamples the number of samples to check - */ - Range findMinMax (int channel, - int startSample, - int numSamples) const noexcept - { - jassert (isPositiveAndBelow (channel, numChannels)); - jassert (startSample >= 0 && startSample + numSamples <= size); - - if (isClear) - return Range(); - - return FloatVectorOperations::findMinAndMax (channels [channel] + startSample, numSamples); - } - - - /** Finds the highest absolute sample value within a region of a channel. */ - float getMagnitude (int channel, - int startSample, - int numSamples) const noexcept - { - jassert (isPositiveAndBelow (channel, numChannels)); - jassert (startSample >= 0 && startSample + numSamples <= size); - - if (isClear) - return 0.0f; - - const Range r (findMinMax (channel, startSample, numSamples)); - - return jmax (r.getStart(), -r.getStart(), r.getEnd(), -r.getEnd()); - } - - /** Finds the highest absolute sample value within a region on all channels. */ - float getMagnitude (int startSample, - int numSamples) const noexcept - { - float mag = 0.0f; - - if (! isClear) - for (int i = 0; i < numChannels; ++i) - mag = jmax (mag, getMagnitude (i, startSample, numSamples)); - - return mag; - } - - /** Returns the root mean squared level for a region of a channel. */ - float getRMSLevel (int channel, - int startSample, - int numSamples) const noexcept - { - jassert (isPositiveAndBelow (channel, numChannels)); - jassert (startSample >= 0 && startSample + numSamples <= size); - - if (numSamples <= 0 || channel < 0 || channel >= numChannels || isClear) - return 0.0f; - - const float* const data = channels [channel] + startSample; - double sum = 0.0; - - for (int i = 0; i < numSamples; ++i) - { - const float sample = data [i]; - sum += sample * sample; - } - - return (float) std::sqrt (sum / numSamples); - } - - /** Reverses a part of a channel. */ - void reverse (int channel, int startSample, int numSamples) const noexcept - { - jassert (isPositiveAndBelow (channel, numChannels)); - jassert (startSample >= 0 && startSample + numSamples <= size); - - if (! isClear) - std::reverse (channels[channel] + startSample, - channels[channel] + startSample + numSamples); - } - - /** Reverses a part of the buffer. */ - void reverse (int startSample, int numSamples) const noexcept - { - for (int i = 0; i < numChannels; ++i) - reverse (i, startSample, numSamples); - } -#endif - private: //============================================================================== int numChannels, size; size_t allocatedBytes; float** channels; - HeapBlock allocatedData; + HeapBlock allocatedData; float* preallocatedChannelSpace [32]; bool isClear; - void allocateData() + bool allocateData() { const size_t channelListSize = sizeof (float*) * (size_t) (numChannels + 1); - allocatedBytes = (size_t) numChannels * (size_t) size * sizeof (float) + channelListSize + 32; - allocatedData.malloc (allocatedBytes); + const size_t nextAllocatedBytes = (size_t) numChannels * (size_t) size * sizeof (float) + channelListSize + 32; + CARLA_SAFE_ASSERT_RETURN (allocatedData.malloc (nextAllocatedBytes), false); + + allocatedBytes = nextAllocatedBytes; channels = reinterpret_cast (allocatedData.getData()); float* chan = (float*) (allocatedData + channelListSize); @@ -1065,11 +976,12 @@ private: channels [numChannels] = nullptr; isClear = false; + return true; } - void allocateChannels (float* const* const dataToReferTo, int offset) + bool allocateChannels (float* const* const dataToReferTo, int offset) { - jassert (offset >= 0); + CARLA_SAFE_ASSERT_RETURN (offset >= 0, false); // (try to avoid doing a malloc here, as that'll blow up things like Pro-Tools) if (numChannels < (int) numElementsInArray (preallocatedChannelSpace)) @@ -1078,20 +990,21 @@ private: } else { - allocatedData.malloc ((size_t) numChannels + 1, sizeof (float*)); + CARLA_SAFE_ASSERT_RETURN( allocatedData.malloc ((size_t) numChannels + 1, sizeof (float*)), false); channels = reinterpret_cast (allocatedData.getData()); } for (int i = 0; i < numChannels; ++i) { // you have to pass in the same number of valid pointers as numChannels - jassert (dataToReferTo[i] != nullptr); + CARLA_SAFE_ASSERT_CONTINUE (dataToReferTo[i] != nullptr); channels[i] = dataToReferTo[i] + offset; } channels [numChannels] = nullptr; isClear = false; + return true; } }; diff --git a/source/modules/water/containers/Array.h b/source/modules/water/containers/Array.h index e4cb7d47e..00282113d 100644 --- a/source/modules/water/containers/Array.h +++ b/source/modules/water/containers/Array.h @@ -68,10 +68,10 @@ public: /** Creates a copy of another array. @param other the array to copy */ - Array (const Array& other) + Array (const Array& other) noexcept { + CARLA_SAFE_ASSERT_RETURN(data.setAllocatedSize (other.numUsed),); numUsed = other.numUsed; - data.setAllocatedSize (other.numUsed); for (int i = 0; i < numUsed; ++i) new (data.elements + i) ElementType (other.data.elements[i]); @@ -91,10 +91,12 @@ public: @param values the array to copy from */ template - explicit Array (const TypeToCreateFrom* values) : numUsed (0) + explicit Array (const TypeToCreateFrom* values) noexcept : numUsed (0) { while (*values != TypeToCreateFrom()) - add (*values++); + { + CARLA_SAFE_ASSERT_BREAK(add (*values++)); + } } /** Initalises from a C array of values. @@ -103,16 +105,16 @@ public: @param numValues the number of values in the array */ template - Array (const TypeToCreateFrom* values, int numValues) : numUsed (numValues) + Array (const TypeToCreateFrom* values, int numValues) noexcept : numUsed (numValues) { - data.setAllocatedSize (numValues); + CARLA_SAFE_ASSERT_RETURN(data.setAllocatedSize (numValues),); for (int i = 0; i < numValues; ++i) new (data.elements + i) ElementType (values[i]); } /** Destructor. */ - ~Array() + ~Array() noexcept { deleteAllElements(); } @@ -120,7 +122,7 @@ public: /** Copies another array. @param other the array to copy */ - Array& operator= (const Array& other) + Array& operator= (const Array& other) noexcept { if (this != &other) { @@ -180,7 +182,7 @@ public: @see clearQuick */ - void clear() + void clear() noexcept { deleteAllElements(); data.setAllocatedSize (0); @@ -190,7 +192,7 @@ public: /** Removes all elements from the array without freeing the array's allocated storage. @see clear */ - void clearQuick() + void clearQuick() noexcept { deleteAllElements(); numUsed = 0; @@ -365,10 +367,13 @@ public: @param newElement the new object to add to the array @see set, insert, addIfNotAlreadyThere, addSorted, addUsingDefaultSort, addArray */ - void add (const ElementType& newElement) + bool add (const ElementType& newElement) noexcept { - data.ensureAllocatedSize (numUsed + 1); + if (! data.ensureAllocatedSize (numUsed + 1)) + return false; + new (data.elements + numUsed++) ElementType (newElement); + return true; } #if WATER_COMPILER_SUPPORTS_MOVE_SEMANTICS @@ -377,10 +382,13 @@ public: @param newElement the new object to add to the array @see set, insert, addIfNotAlreadyThere, addSorted, addUsingDefaultSort, addArray */ - void add (ElementType&& newElement) + bool add (ElementType&& newElement) noexcept { - data.ensureAllocatedSize (numUsed + 1); + if (! data.ensureAllocatedSize (numUsed + 1)) + return false; + new (data.elements + numUsed++) ElementType (static_cast (newElement)); + return true; } #endif @@ -396,10 +404,10 @@ public: @param newElement the new object to add to the array @see add, addSorted, addUsingDefaultSort, set */ - void insert (int indexToInsertAt, ParameterType newElement) + bool insert (int indexToInsertAt, ParameterType newElement) noexcept { - data.ensureAllocatedSize (numUsed + 1); - jassert (data.elements != nullptr); + if (! data.ensureAllocatedSize (numUsed + 1)) + return false; if (isPositiveAndBelow (indexToInsertAt, numUsed)) { @@ -407,7 +415,7 @@ public: const int numberToMove = numUsed - indexToInsertAt; if (numberToMove > 0) - memmove (insertPos + 1, insertPos, ((size_t) numberToMove) * sizeof (ElementType)); + std::memmove (insertPos + 1, insertPos, ((size_t) numberToMove) * sizeof (ElementType)); new (insertPos) ElementType (newElement); ++numUsed; @@ -416,6 +424,8 @@ public: { new (data.elements + numUsed++) ElementType (newElement); } + + return true; } /** Inserts multiple copies of an element into the array at a given position. @@ -513,8 +523,7 @@ public: if (contains (newElement)) return false; - add (newElement); - return true; + return add (newElement); } /** Replaces an element with a new value. @@ -1035,9 +1044,9 @@ public: removing elements, they may have quite a lot of unused space allocated. This method will reduce the amount of allocated storage to a minimum. */ - void minimiseStorageOverheads() + bool minimiseStorageOverheads() noexcept { - data.shrinkToNoMoreThan (numUsed); + return data.shrinkToNoMoreThan (numUsed); } /** Increases the array's internal storage to hold a minimum number of elements. @@ -1046,9 +1055,9 @@ public: the array won't have to keep dynamically resizing itself as the elements are added, and it'll therefore be more efficient. */ - void ensureStorageAllocated (const int minNumElements) + bool ensureStorageAllocated (const int minNumElements) noexcept { - data.ensureAllocatedSize (minNumElements); + return data.ensureAllocatedSize (minNumElements); } //============================================================================== diff --git a/source/modules/water/containers/ArrayAllocationBase.h b/source/modules/water/containers/ArrayAllocationBase.h index 4d63ca149..6f56ba563 100644 --- a/source/modules/water/containers/ArrayAllocationBase.h +++ b/source/modules/water/containers/ArrayAllocationBase.h @@ -39,9 +39,6 @@ namespace water { This class isn't really for public use - it's used by the other array classes, but might come in handy for some purposes. - It inherits from a critical section class to allow the arrays to use - the "empty base class optimisation" pattern to reduce their footprint. - @see Array, OwnedArray, ReferenceCountedArray */ template @@ -83,17 +80,24 @@ public: @param numElements the number of elements that are needed */ - void setAllocatedSize (const int numElements) + bool setAllocatedSize (const int numElements) noexcept { if (numAllocated != numElements) { if (numElements > 0) - elements.realloc ((size_t) numElements); + { + if (! elements.realloc ((size_t) numElements)) + return false; + } else + { elements.free(); + } numAllocated = numElements; } + + return true; } /** Increases the amount of storage allocated if it is less than a given amount. @@ -104,21 +108,23 @@ public: @param minNumElements the minimum number of elements that are needed */ - void ensureAllocatedSize (const int minNumElements) + bool ensureAllocatedSize (const int minNumElements) noexcept { if (minNumElements > numAllocated) - setAllocatedSize ((minNumElements + minNumElements / 2 + 8) & ~7); + return setAllocatedSize ((minNumElements + minNumElements / 2 + 8) & ~7); - jassert (numAllocated <= 0 || elements != nullptr); + return true; } /** Minimises the amount of storage allocated so that it's no more than the given number of elements. */ - void shrinkToNoMoreThan (const int maxNumElements) + bool shrinkToNoMoreThan (const int maxNumElements) noexcept { if (maxNumElements < numAllocated) - setAllocatedSize (maxNumElements); + return setAllocatedSize (maxNumElements); + + return true; } /** Swap the contents of two objects. */ diff --git a/source/modules/water/containers/OwnedArray.h b/source/modules/water/containers/OwnedArray.h index 4a1bcb974..d659c4d25 100644 --- a/source/modules/water/containers/OwnedArray.h +++ b/source/modules/water/containers/OwnedArray.h @@ -271,8 +271,9 @@ public: */ ObjectClass* add (ObjectClass* newObject) noexcept { - data.ensureAllocatedSize (numUsed + 1); - jassert (data.elements != nullptr); + if (! data.ensureAllocatedSize (numUsed + 1)) + return nullptr; + data.elements [numUsed++] = newObject; return newObject; } @@ -303,14 +304,14 @@ public: if (indexToInsertAt > numUsed) indexToInsertAt = numUsed; - data.ensureAllocatedSize (numUsed + 1); - jassert (data.elements != nullptr); + if (! data.ensureAllocatedSize (numUsed + 1)) + return nullptr; ObjectClass** const e = data.elements + indexToInsertAt; const int numToMove = numUsed - indexToInsertAt; if (numToMove > 0) - memmove (e + 1, e, sizeof (ObjectClass*) * (size_t) numToMove); + std::memmove (e + 1, e, sizeof (ObjectClass*) * (size_t) numToMove); *e = newObject; ++numUsed; @@ -369,8 +370,7 @@ public: if (contains (newObject)) return false; - add (newObject); - return true; + return add (newObject) != nullptr; } /** Replaces an object in the array with a different one. @@ -771,9 +771,9 @@ public: removing elements, they may have quite a lot of unused space allocated. This method will reduce the amount of allocated storage to a minimum. */ - void minimiseStorageOverheads() noexcept + bool minimiseStorageOverheads() noexcept { - data.shrinkToNoMoreThan (numUsed); + return data.shrinkToNoMoreThan (numUsed); } /** Increases the array's internal storage to hold a minimum number of elements. @@ -782,9 +782,9 @@ public: the array won't have to keep dynamically resizing itself as the elements are added, and it'll therefore be more efficient. */ - void ensureStorageAllocated (const int minNumElements) noexcept + bool ensureStorageAllocated (const int minNumElements) noexcept { - data.ensureAllocatedSize (minNumElements); + return data.ensureAllocatedSize (minNumElements); } //============================================================================== diff --git a/source/modules/water/memory/HeapBlock.h b/source/modules/water/memory/HeapBlock.h index f7b3401db..322d1e7fd 100644 --- a/source/modules/water/memory/HeapBlock.h +++ b/source/modules/water/memory/HeapBlock.h @@ -70,14 +70,9 @@ namespace water { as their less object-oriented counterparts. Despite adding safety, you probably won't sacrifice any performance by using this in place of normal pointers. - The throwOnFailure template parameter can be set to true if you'd like the class - to throw a std::bad_alloc exception when an allocation fails. If this is false, - then a failed allocation will just leave the heapblock with a null pointer (assuming - that the system's malloc() function doesn't throw). - @see Array, OwnedArray, MemoryBlock */ -template +template class HeapBlock { public: @@ -181,7 +176,7 @@ public: The data that is allocated will be freed when this object is deleted, or when you call free() or any of the allocation methods. */ - bool malloc (const size_t newNumElements, const size_t elementSize = sizeof (ElementType)) + bool malloc (const size_t newNumElements, const size_t elementSize = sizeof (ElementType)) noexcept { std::free (data); data = static_cast (std::malloc (newNumElements * elementSize)); @@ -192,7 +187,7 @@ public: /** Allocates a specified amount of memory and clears it. This does the same job as the malloc() method, but clears the memory that it allocates. */ - bool calloc (const size_t newNumElements, const size_t elementSize = sizeof (ElementType)) + bool calloc (const size_t newNumElements, const size_t elementSize = sizeof (ElementType)) noexcept { std::free (data); data = static_cast (std::calloc (newNumElements, elementSize)); @@ -204,7 +199,7 @@ public: This does the same job as either malloc() or calloc(), depending on the initialiseToZero parameter. */ - bool allocate (const size_t newNumElements, bool initialiseToZero) + bool allocate (const size_t newNumElements, bool initialiseToZero) noexcept { std::free (data); data = static_cast (initialiseToZero @@ -219,7 +214,7 @@ public: The semantics of this method are the same as malloc() and calloc(), but it uses realloc() to keep as much of the existing data as possible. */ - bool realloc (const size_t newNumElements, const size_t elementSize = sizeof (ElementType)) + bool realloc (const size_t newNumElements, const size_t elementSize = sizeof (ElementType)) noexcept { data = static_cast (data == nullptr ? std::malloc (newNumElements * elementSize) : std::realloc (data, newNumElements * elementSize)); @@ -238,8 +233,7 @@ public: /** Swaps this object's data with the data of another HeapBlock. The two objects simply exchange their data pointers. */ - template - void swapWith (HeapBlock& other) noexcept + void swapWith (HeapBlock& other) noexcept { std::swap (data, other.data); } diff --git a/source/modules/water/misc/Time.cpp b/source/modules/water/misc/Time.cpp index 27847a749..cd1e73c87 100644 --- a/source/modules/water/misc/Time.cpp +++ b/source/modules/water/misc/Time.cpp @@ -24,6 +24,7 @@ */ #include "Time.h" +#include "../memory/Atomic.h" #include #include @@ -38,7 +39,7 @@ namespace water { namespace TimeHelpers { - static uint32 lastMSCounterValue = 0; + static Atomic lastMSCounterValue; #ifdef CARLA_OS_MAC /* NB: these are kept outside the HiResCounterInfo struct and initialised to 1 to avoid @@ -129,12 +130,12 @@ uint32 Time::getMillisecondCounter() noexcept { const uint32 now = water_millisecondsSinceStartup(); - if (now < TimeHelpers::lastMSCounterValue) + if (now < TimeHelpers::lastMSCounterValue.get()) { // in multi-threaded apps this might be called concurrently, so // make sure that our last counter value only increases and doesn't // go backwards.. - if (now < TimeHelpers::lastMSCounterValue - 1000) + if (now < TimeHelpers::lastMSCounterValue.get() - 1000U) TimeHelpers::lastMSCounterValue = now; } else @@ -147,10 +148,8 @@ uint32 Time::getMillisecondCounter() noexcept uint32 Time::getApproximateMillisecondCounter() noexcept { - if (TimeHelpers::lastMSCounterValue == 0) - getMillisecondCounter(); - - return TimeHelpers::lastMSCounterValue; + const uint32 t = TimeHelpers::lastMSCounterValue.get(); + return t == 0 ? getMillisecondCounter() : t; } }