From 056337b5f049af320b17cdfbed6cb4602679f425 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Mon, 2 Dec 2019 12:22:02 +0100 Subject: [PATCH 1/6] Disabled the move semantics from Buffer, just in case. Disabled the move semantics test --- src/sfizz/Buffer.h | 52 ++++++++++++++++++++++++---------------------- tests/BufferT.cpp | 10 ++++----- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/sfizz/Buffer.h b/src/sfizz/Buffer.h index 60b53e19..c790be3f 100644 --- a/src/sfizz/Buffer.h +++ b/src/sfizz/Buffer.h @@ -147,18 +147,19 @@ public: * * @param other */ - Buffer(Buffer&& other) - { - if (this != &other) { - std::free(paddedData); - largerSize = std::exchange(other.largerSize, 0); - alignedSize = std::exchange(other.alignedSize, 0); - paddedData = std::exchange(other.paddedData, nullptr); - normalData = std::exchange(other.normalData, nullptr); - normalEnd = std::exchange(other.normalEnd, nullptr); - _alignedEnd = std::exchange(other._alignedEnd, nullptr); - } - } + Buffer(Buffer&& other) = delete; + // { + // if (this != &other) { + // counter().bufferDeleted(largerSize * sizeof(value_type)); + // std::free(paddedData); + // largerSize = std::exchange(other.largerSize, 0); + // alignedSize = std::exchange(other.alignedSize, 0); + // paddedData = std::exchange(other.paddedData, nullptr); + // normalData = std::exchange(other.normalData, nullptr); + // normalEnd = std::exchange(other.normalEnd, nullptr); + // _alignedEnd = std::exchange(other._alignedEnd, nullptr); + // } + // } Buffer& operator=(const Buffer& other) { @@ -169,19 +170,20 @@ public: return *this; } - Buffer& operator=(Buffer&& other) - { - if (this != &other) { - std::free(paddedData); - largerSize = std::exchange(other.largerSize, 0); - alignedSize = std::exchange(other.alignedSize, 0); - paddedData = std::exchange(other.paddedData, nullptr); - normalData = std::exchange(other.normalData, nullptr); - normalEnd = std::exchange(other.normalEnd, nullptr); - _alignedEnd = std::exchange(other._alignedEnd, nullptr); - } - return *this; - } + Buffer& operator=(Buffer&& other) = delete; + // { + // if (this != &other) { + // counter().bufferDeleted(largerSize * sizeof(value_type)); + // std::free(paddedData); + // largerSize = std::exchange(other.largerSize, 0); + // alignedSize = std::exchange(other.alignedSize, 0); + // paddedData = std::exchange(other.paddedData, nullptr); + // normalData = std::exchange(other.normalData, nullptr); + // normalEnd = std::exchange(other.normalEnd, nullptr); + // _alignedEnd = std::exchange(other._alignedEnd, nullptr); + // } + // return *this; + // } Type& operator[](int idx) { return *(normalData + idx); } constexpr pointer data() const noexcept { return normalData; } diff --git a/tests/BufferT.cpp b/tests/BufferT.cpp index 9c79ae22..873d39b9 100644 --- a/tests/BufferT.cpp +++ b/tests/BufferT.cpp @@ -157,8 +157,8 @@ TEST_CASE("[Buffer] Copy and move") checkBoundaries(copyConstructed, baseSize); REQUIRE(std::all_of(copyConstructed.begin(), copyConstructed.end(), [](auto value) { return value == 1.0f; })); - sfz::Buffer moveConstructed { std::move(buffer) }; - REQUIRE(buffer.empty()); - checkBoundaries(moveConstructed, baseSize); - REQUIRE(std::all_of(moveConstructed.begin(), moveConstructed.end(), [](auto value) { return value == 1.0f; })); -} \ No newline at end of file + // sfz::Buffer moveConstructed { std::move(buffer) }; + // REQUIRE(buffer.empty()); + // checkBoundaries(moveConstructed, baseSize); + // REQUIRE(std::all_of(moveConstructed.begin(), moveConstructed.end(), [](auto value) { return value == 1.0f; })); +} From d849d4f8b79a438ad96911bcf99b88b8fd692c61 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Mon, 2 Dec 2019 12:22:21 +0100 Subject: [PATCH 2/6] Added a buffer counting to the synth in order to track memory --- src/sfizz/Buffer.h | 81 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 71 insertions(+), 10 deletions(-) diff --git a/src/sfizz/Buffer.h b/src/sfizz/Buffer.h index c790be3f..60b87d3c 100644 --- a/src/sfizz/Buffer.h +++ b/src/sfizz/Buffer.h @@ -29,20 +29,65 @@ #include #include #include - +#ifdef DEBUG +#include +#endif namespace sfz { + /** - * @brief A heap buffer structure that tries to align its beginning and adds a small offset - * at the end for alignment too. + * @brief A buffer counting class that tries to track the memory usage. + */ +class BufferCounter +{ +public: + BufferCounter() = default; + ~BufferCounter() + { +#ifdef DEBUG + std::cout << "Remaining buffers on destruction: " << numBuffers << '\n'; + std::cout << "Total size: " << bytes << '\n'; +#endif + } + + void newBuffer(int size) + { + numBuffers++; + bytes.fetch_add(size); + } + + void bufferResized(int oldSize, int newSize) + { + bytes.fetch_add(newSize); + bytes.fetch_sub(oldSize); + } + + void bufferDeleted(int size) + { + numBuffers--; + bytes.fetch_sub(size); + } + + int getNumBuffers() { return numBuffers; } + int getTotalBytes() { return bytes; } +private: + std::atomic numBuffers { 0 }; + std::atomic bytes { 0 }; +}; + +/** + * @brief A heap buffer structure that tries to align its beginning and + * adds a small offset at the end for alignment too. * - * Apparently on Linux this effort is mostly useless, and in the end most of the SIMD operations - * are coded with alignment checks and sentinels so this class could probably be much simpler. - * It does however wrap realloc which in some cases should be a bit more efficient than - * allocating a whole new block. + * Apparently on Linux this effort is mostly useless, and in the end + * most of the SIMD operations are coded with alignment checks and + * sentinels so this class could probably be much simpler. It does + * however wrap realloc which in some cases should be a bit more + * efficient than allocating a whole new block. * - * @tparam Type The buffer type - * @tparam Alignment the required alignment in bytes (defaults to SIMDConfig::defaultAlignment) + * @tparam Type The buffer type + * @tparam Alignment the required alignment in bytes (defaults to + * SIMDConfig::defaultAlignment) */ template class Buffer { @@ -64,7 +109,7 @@ public: */ Buffer() { - + counter().newBuffer(0); } /** @@ -74,6 +119,7 @@ public: */ Buffer(size_t size) { + counter().newBuffer(0); resize(size); } @@ -98,6 +144,7 @@ public: return false; } + counter().bufferResized(largerSize * sizeof(value_type), tempSize * sizeof(value_type)); largerSize = tempSize; alignedSize = newSize; paddedData = static_cast(newData); @@ -118,6 +165,7 @@ public: */ void clear() { + counter().bufferResized(largerSize * sizeof(value_type), 0); largerSize = 0; alignedSize = 0; std::free(paddedData); @@ -127,6 +175,7 @@ public: } ~Buffer() { + counter().bufferDeleted(largerSize * sizeof(value_type)); std::free(paddedData); } @@ -194,6 +243,18 @@ public: constexpr pointer alignedEnd() noexcept { return _alignedEnd; } + /** + * @brief Return the buffer counter object. + * + * TODO: In C++ 17 all of this can be static inline. + * + * @return The buffer counter on which you can call various methods. + */ + static BufferCounter& counter() + { + static BufferCounter counter; + return counter; + } private: static constexpr auto AlignmentMask { Alignment - 1 }; static constexpr auto TypeAlignment { Alignment / sizeof(value_type) }; From 9f96968169459d796941843663e046f04a903cb4 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Mon, 2 Dec 2019 12:24:34 +0100 Subject: [PATCH 3/6] Added an API to access the memory usage --- src/sfizz.h | 20 ++++++++++++++++++++ src/sfizz/Synth.h | 14 ++++++++++++++ src/sfizz/sfizz_wrapper.cpp | 12 ++++++++++++ 3 files changed, 46 insertions(+) diff --git a/src/sfizz.h b/src/sfizz.h index 6df7d7b8..0f155850 100644 --- a/src/sfizz.h +++ b/src/sfizz.h @@ -305,6 +305,26 @@ void sfizz_set_num_voices(sfizz_synth_t* synth, int num_voices); */ int sfizz_get_num_voices(sfizz_synth_t* synth); +/** + * @brief Get the number of allocated buffers from the synth. + * + * @param synth The synth + * + * @return The number of buffers held by the synth + */ +int sfizz_get_num_buffers(sfizz_synth_t* synth); +/** + * @brief Get the number of bytes allocated from the synth. Note that this + * value can be less than the actual memory usage since it only + * counts the buffer objects managed by sfizz. + * + * @param synth The synth + * + * @return The number of bytes held by the synth in buffers; + */ +int sfizz_get_num_bytes(sfizz_synth_t* synth); + + #ifdef __cplusplus } #endif diff --git a/src/sfizz/Synth.h b/src/sfizz/Synth.h index 410c14f4..2f91b189 100644 --- a/src/sfizz/Synth.h +++ b/src/sfizz/Synth.h @@ -310,6 +310,20 @@ public: * @return Oversampling */ uint32_t getPreloadSize() const noexcept; + + /** + * @brief Gets the number of allocated buffers. + * + * @return The allocated buffers. + */ + int getAllocatedBuffers() const noexcept { return Buffer::counter().getNumBuffers(); } + + /** + * @brief Gets the number of bytes allocated through the buffers + * + * @return The allocated bytes. + */ + int getAllocatedBytes() const noexcept { return Buffer::counter().getTotalBytes(); } protected: /** * @brief The parser callback; this is called by the parent object each time diff --git a/src/sfizz/sfizz_wrapper.cpp b/src/sfizz/sfizz_wrapper.cpp index 736241d8..2fccaa34 100644 --- a/src/sfizz/sfizz_wrapper.cpp +++ b/src/sfizz/sfizz_wrapper.cpp @@ -192,6 +192,18 @@ int sfizz_get_num_voices(sfizz_synth_t* synth) return self->getNumVoices(); } +int sfizz_get_num_buffers(sfizz_synth_t* synth) +{ + auto self = reinterpret_cast(synth); + return self->getAllocatedBuffers(); +} + +int sfizz_get_num_bytes(sfizz_synth_t* synth) +{ + auto self = reinterpret_cast(synth); + return self->getAllocatedBytes(); +} + #ifdef __cplusplus } #endif From e69781ff6b2b7711054a1e2b93d8a14093713105 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Mon, 2 Dec 2019 12:24:54 +0100 Subject: [PATCH 4/6] Added logging in the most common clients --- clients/jack_client.cpp | 7 +++++-- lv2/sfizz.c | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/clients/jack_client.cpp b/clients/jack_client.cpp index 1092a487..682d873b 100644 --- a/clients/jack_client.cpp +++ b/clients/jack_client.cpp @@ -273,8 +273,11 @@ int main(int argc, char** argv) signal(SIGQUIT, done); while (!shouldClose){ - // synth.garbageCollect(); - std::this_thread::sleep_for(1s); +#ifdef DEBUG + std::cout << "Allocated buffers: " << synth.getAllocatedBuffers() << '\n'; + std::cout << "Total size: " << synth.getAllocatedBytes() << '\n'; +#endif + std::this_thread::sleep_for(2s); } std::cout << "Closing..." << '\n'; diff --git a/lv2/sfizz.c b/lv2/sfizz.c index f78794ae..749dc9a9 100644 --- a/lv2/sfizz.c +++ b/lv2/sfizz.c @@ -63,6 +63,7 @@ #define DEFAULT_VOICES 64 #define DEFAULT_OVERSAMPLING SFIZZ_OVERSAMPLING_X1 #define DEFAULT_PRELOAD 8192 +#define LOG_SAMPLE_COUNT 96000 #define UNUSED(x) (void)(x) typedef struct @@ -121,6 +122,7 @@ typedef struct sfizz_oversampling_factor_t oversampling; bool changing_state; int max_block_size; + int sample_counter; float sample_rate; } sfizz_plugin_t; @@ -226,6 +228,7 @@ instantiate(const LV2_Descriptor *descriptor, self->oversampling = DEFAULT_OVERSAMPLING; self->preload_size = DEFAULT_PRELOAD; self->changing_state = false; + self->sample_counter = 0; // Get the features from the host and populate the structure for (const LV2_Feature *const *f = features; *f; f++) @@ -575,6 +578,17 @@ run(LV2_Handle instance, uint32_t sample_count) } } +#ifdef DEBUG + // Log the buffer usage + self->sample_counter += (int)sample_count; + if (self->sample_counter > LOG_SAMPLE_COUNT) + { + lv2_log_note(&self->logger, "[run] Allocated buffers: %d\n", sfizz_get_num_buffers(self->synth)); + lv2_log_note(&self->logger, "[run] Allocated bytes: %d\n", sfizz_get_num_bytes(self->synth)); + self->sample_counter -= LOG_SAMPLE_COUNT; + } +#endif + // Render the block sfizz_render_block(self->synth, self->output_buffers, 2, (int)sample_count); } From c00f5174b1e066a01e1426d1e0d433107c16d070 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Mon, 2 Dec 2019 12:25:21 +0100 Subject: [PATCH 5/6] Corrected the bug ; the reloading procedures did not take the current oversampling parameter into account. --- src/sfizz/FilePool.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sfizz/FilePool.cpp b/src/sfizz/FilePool.cpp index 7e9addce..dbda0e44 100644 --- a/src/sfizz/FilePool.cpp +++ b/src/sfizz/FilePool.cpp @@ -173,7 +173,7 @@ void sfz::FilePool::setPreloadSize(uint32_t preloadSize) noexcept { // Update all the preloaded sizes for (auto& preloadedFile : preloadedFiles) { - const auto numFrames = preloadedFile.second.preloadedData->getNumFrames(); + const auto numFrames = preloadedFile.second.preloadedData->getNumFrames() / oversamplingFactor; const uint32_t maxOffset = numFrames > this->preloadSize ? numFrames - this->preloadSize : 0; fs::path file { rootDirectory / std::string(preloadedFile.first) }; SndfileHandle sndFile(reinterpret_cast(file.c_str())); @@ -246,7 +246,7 @@ void sfz::FilePool::setOversamplingFactor(sfz::Oversampling factor) noexcept { float samplerateChange { static_cast(factor) / static_cast(this->oversamplingFactor) }; for (auto& preloadedFile : preloadedFiles) { - const auto numFrames = preloadedFile.second.preloadedData->getNumFrames(); + const auto numFrames = preloadedFile.second.preloadedData->getNumFrames() / this->oversamplingFactor; const uint32_t maxOffset = numFrames > this->preloadSize ? numFrames - this->preloadSize : 0; fs::path file { rootDirectory / std::string(preloadedFile.first) }; SndfileHandle sndFile(reinterpret_cast(file.c_str())); From 78b90350a857c1a1736fb073c57dd1bf6b488b15 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Mon, 2 Dec 2019 12:35:14 +0100 Subject: [PATCH 6/6] Moved the memory logging to NDEBUG --- clients/jack_client.cpp | 2 +- lv2/sfizz.c | 2 +- src/sfizz/Buffer.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/jack_client.cpp b/clients/jack_client.cpp index 682d873b..619cb9d7 100644 --- a/clients/jack_client.cpp +++ b/clients/jack_client.cpp @@ -273,7 +273,7 @@ int main(int argc, char** argv) signal(SIGQUIT, done); while (!shouldClose){ -#ifdef DEBUG +#ifndef NDEBUG std::cout << "Allocated buffers: " << synth.getAllocatedBuffers() << '\n'; std::cout << "Total size: " << synth.getAllocatedBytes() << '\n'; #endif diff --git a/lv2/sfizz.c b/lv2/sfizz.c index 749dc9a9..90df55eb 100644 --- a/lv2/sfizz.c +++ b/lv2/sfizz.c @@ -578,7 +578,7 @@ run(LV2_Handle instance, uint32_t sample_count) } } -#ifdef DEBUG +#ifndef NDEBUG // Log the buffer usage self->sample_counter += (int)sample_count; if (self->sample_counter > LOG_SAMPLE_COUNT) diff --git a/src/sfizz/Buffer.h b/src/sfizz/Buffer.h index 60b87d3c..81df042f 100644 --- a/src/sfizz/Buffer.h +++ b/src/sfizz/Buffer.h @@ -44,7 +44,7 @@ public: BufferCounter() = default; ~BufferCounter() { -#ifdef DEBUG +#ifndef NDEBUG std::cout << "Remaining buffers on destruction: " << numBuffers << '\n'; std::cout << "Total size: " << bytes << '\n'; #endif