From f2b334c5677b949be9133dee225f7fb28ec6e463 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Mon, 15 Jun 2020 10:59:12 +0200 Subject: [PATCH] Implement helpers for the swap and pop idioms This code is repeated from place to place, I figured having a central place to put it and test it was OK. --- src/sfizz/EQPool.cpp | 13 +---- src/sfizz/FilePool.cpp | 33 +++-------- src/sfizz/FilePool.h | 14 +++-- src/sfizz/FilterPool.cpp | 13 +---- src/sfizz/SwapAndPop.h | 98 +++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/SwapAndPopT.cpp | 123 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 242 insertions(+), 53 deletions(-) create mode 100644 src/sfizz/SwapAndPop.h create mode 100644 tests/SwapAndPopT.cpp diff --git a/src/sfizz/EQPool.cpp b/src/sfizz/EQPool.cpp index adfdfec7..6d3d75b2 100644 --- a/src/sfizz/EQPool.cpp +++ b/src/sfizz/EQPool.cpp @@ -2,6 +2,7 @@ #include #include "absl/algorithm/container.h" #include "SIMDHelpers.h" +#include "SwapAndPop.h" sfz::EQHolder::EQHolder(const MidiState& state) :midiState(state) @@ -133,18 +134,8 @@ size_t sfz::EQPool::setnumEQs(size_t numEQs) { const std::lock_guard eqLock { eqGuard }; - auto eqIterator = eqs.begin(); - auto eqSentinel = eqs.rbegin(); - while (eqIterator < eqSentinel.base()) { - if (eqIterator->use_count() == 1) { - std::iter_swap(eqIterator, eqSentinel); - ++eqSentinel; - } else { - ++eqIterator; - } - } + swapAndPopAll(eqs, [](sfz::EQHolderPtr& eq) { return eq.use_count() == 1; }); - eqs.resize(std::distance(eqs.begin(), eqSentinel.base())); for (size_t i = eqs.size(); i < numEQs; ++i) { eqs.emplace_back(std::make_shared(midiState)); eqs.back()->setSampleRate(sampleRate); diff --git a/src/sfizz/FilePool.cpp b/src/sfizz/FilePool.cpp index d0ac449e..04f33cc9 100644 --- a/src/sfizz/FilePool.cpp +++ b/src/sfizz/FilePool.cpp @@ -28,6 +28,7 @@ #include "Buffer.h" #include "AudioBuffer.h" #include "AudioSpan.h" +#include "SwapAndPop.h" #include "Config.h" #include "Debug.h" #include "Oversampler.h" @@ -431,37 +432,19 @@ void sfz::FilePool::cleanupPromises() noexcept // The garbage collection cleared the data from these so we can move them // back to the empty queue - auto clearedIterator = promisesToClear.begin(); - auto clearedSentinel = promisesToClear.rbegin(); - while (clearedIterator < clearedSentinel.base()) { - if (clearedIterator->get()->dataStatus == FilePromise::DataStatus::Wait) { - emptyPromises.push_back(*clearedIterator); - std::iter_swap(clearedIterator, clearedSentinel); - ++clearedSentinel; - } else { - ++clearedIterator; - } - } - promisesToClear.resize(std::distance(promisesToClear.begin(), clearedSentinel.base())); + auto promiseWaiting = [](FilePromisePtr& p) { return p->waiting(); }; + auto moveToEmpty = [&](FilePromisePtr& p) { return emptyPromises.push_back(p); }; + swapAndPopAll(promisesToClear, promiseWaiting, moveToEmpty); - FilePromisePtr promise; // Remove the promises from the filled queue and put them in a linear // storage + FilePromisePtr promise; while (filledPromiseQueue.try_pop(promise)) temporaryFilePromises.push_back(promise); - auto filledIterator = temporaryFilePromises.begin(); - auto filledSentinel = temporaryFilePromises.rbegin(); - while (filledIterator < filledSentinel.base()) { - if (filledIterator->use_count() == 1) { - promisesToClear.push_back(*filledIterator); - std::iter_swap(filledIterator, filledSentinel); - ++filledSentinel; - } else { - ++filledIterator; - } - } - temporaryFilePromises.resize(std::distance(temporaryFilePromises.begin(), filledSentinel.base())); + auto promiseUsedOnce = [](FilePromisePtr& p) { return p.use_count() == 1; }; + auto moveToClear = [&](FilePromisePtr& p) { return promisesToClear.push_back(p); }; + swapAndPopAll(temporaryFilePromises, promiseUsedOnce, moveToClear); } void sfz::FilePool::setOversamplingFactor(sfz::Oversampling factor) noexcept diff --git a/src/sfizz/FilePool.h b/src/sfizz/FilePool.h index 70973a87..b542e8bb 100644 --- a/src/sfizz/FilePool.h +++ b/src/sfizz/FilePool.h @@ -85,18 +85,20 @@ struct FilePromise sampleRate = config::defaultSampleRate; } - void waitCompletion() - { - while (dataStatus == DataStatus::Wait) - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - } - enum class DataStatus { Wait = 0, Ready, Error, }; + bool waiting() const { return dataStatus == DataStatus::Wait; } + + void sleepUntilComplete() + { + while (waiting()) + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + FileId fileId {}; FileAudioBufferPtr preloadedData {}; FileAudioBuffer fileData {}; diff --git a/src/sfizz/FilterPool.cpp b/src/sfizz/FilterPool.cpp index 3b61276f..164b3ea8 100644 --- a/src/sfizz/FilterPool.cpp +++ b/src/sfizz/FilterPool.cpp @@ -1,5 +1,6 @@ #include "FilterPool.h" #include "SIMDHelpers.h" +#include "SwapAndPop.h" #include "absl/algorithm/container.h" #include #include @@ -134,18 +135,8 @@ size_t sfz::FilterPool::setNumFilters(size_t numFilters) { const std::lock_guard filterLock { filterGuard }; - auto filterIterator = filters.begin(); - auto filterSentinel = filters.rbegin(); - while (filterIterator < filterSentinel.base()) { - if (filterIterator->use_count() == 1) { - std::iter_swap(filterIterator, filterSentinel); - ++filterSentinel; - } else { - ++filterIterator; - } - } + swapAndPopAll(filters, [](sfz::FilterHolderPtr& filter) { return filter.use_count() == 1; }); - filters.resize(std::distance(filters.begin(), filterSentinel.base())); for (size_t i = filters.size(); i < numFilters; ++i) { filters.emplace_back(std::make_shared(midiState)); filters.back()->setSampleRate(sampleRate); diff --git a/src/sfizz/SwapAndPop.h b/src/sfizz/SwapAndPop.h new file mode 100644 index 00000000..e9a5d1f2 --- /dev/null +++ b/src/sfizz/SwapAndPop.h @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: BSD-2-Clause + +// This code is part of the sfizz library and is licensed under a BSD 2-clause +// license. You should have receive a LICENSE.md file along with the code. +// If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz + +#pragma once + +namespace sfz +{ + +/** + * @brief Implement a swap and pop idiom for a vector, applying an action on elements + * fitting a condition and removing them by swapping them out with the last element of + * the vector. This costs less than `erase` at the cost of the ordering of the vector. + * + * @param vector + * @param condition which elements to remove + * @param action what to do with them before removing + * @return unsigned + */ +template +inline unsigned swapAndPopAll(std::vector& vector, F&& condition, A&& action) +{ + auto it = vector.begin(); + auto sentinel = vector.rbegin(); + while (it < sentinel.base()) { + if (condition(*it)) { + action(*it); + std::iter_swap(it, sentinel); + ++sentinel; + } else { + ++it; + } + } + auto removed = std::distance(sentinel.base(), vector.end()); + vector.resize(std::distance(vector.begin(), sentinel.base())); + return removed; +} + +/** + * @brief Implement a swap and pop idiom for a vector, removing all the elements + * fitting some condition by swapping them out with the last element of the vector. + * This costs less than `erase` at the cost of the ordering of the vector. + * + * @param vector + * @param condition which elements to remove + * @return unsigned + */ +template +inline unsigned swapAndPopAll(std::vector& vector, F&& condition) +{ + return swapAndPopAll(vector, std::forward(condition), [](T& v){ (void)v; }); +} + +/** + * @brief Implement a swap and pop idiom for a vector, applying an action on the first + * element fitting a condition and removing it by swapping it out with the last element of + * the vector. This costs less than `erase` at the cost of the ordering of the vector. + * + * @param vector + * @param condition which element to remove + * @param action what to do with it before removing + * @return unsigned + */ +template +inline bool swapAndPopFirst(std::vector& vector, F&& condition, A&& action) +{ + auto it = vector.begin(); + auto sentinel = vector.rbegin(); + while (it < sentinel.base()) { + if (condition(*it)) { + action(*it); + std::iter_swap(it, sentinel); + vector.pop_back(); + return true; + } + ++it; + } + return false; +} + +/** + * @brief Implement a swap and pop idiom for a vector, removing the first element + * fitting some condition by swapping it out with the last element of the vector. + * This costs less than `erase` at the cost of the ordering of the vector. + * + * @param vector + * @param condition which element to remove + * @return unsigned + */ +template +inline unsigned swapAndPopFirst(std::vector& vector, F&& condition) +{ + return swapAndPopFirst(vector, std::forward(condition), [](T& v){ (void)v; }); +} + +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c71cee5b..71cf564a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -31,6 +31,7 @@ set(SFIZZ_TEST_SOURCES FloatHelpersT.cpp WavetablesT.cpp SemaphoreT.cpp + SwapAndPopT.cpp TuningT.cpp ) diff --git a/tests/SwapAndPopT.cpp b/tests/SwapAndPopT.cpp new file mode 100644 index 00000000..dd06d65d --- /dev/null +++ b/tests/SwapAndPopT.cpp @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: BSD-2-Clause + +// This code is part of the sfizz library and is licensed under a BSD 2-clause +// license. You should have receive a LICENSE.md file along with the code. +// If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz + +#include "catch2/catch.hpp" +#include "sfizz/SwapAndPop.h" +using namespace Catch::literals; + +TEST_CASE("[SwapAndPop] Popping one element") +{ + SECTION("PopFirst -- Test 1") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 2; })); + std::vector expected { 1, 6, 3, 4, 5 }; + REQUIRE( vector == expected ); + } + SECTION("PopFirst -- Test 2") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 5; })); + std::vector expected { 1, 2, 3, 4, 6 }; + REQUIRE( vector == expected ); + } + SECTION("PopFirst -- Test 3") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 1; })); + std::vector expected { 6, 2, 3, 4, 5 }; + REQUIRE( vector == expected ); + } + SECTION("PopFirst -- Test 4") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 6; })); + std::vector expected { 1, 2, 3, 4, 5 }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 1") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 2; }) == 1); + std::vector expected { 1, 6, 3, 4, 5 }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 2") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 5; }) == 1); + std::vector expected { 1, 2, 3, 4, 6 }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 3") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 1; }) == 1); + std::vector expected { 6, 2, 3, 4, 5 }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 4") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 6; }) == 1); + std::vector expected { 1, 2, 3, 4, 5 }; + REQUIRE( vector == expected ); + } +} + +TEST_CASE("[SwapAndPop] Popping multiple elements") +{ + SECTION("PopAll -- Test 1") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 1 || v == 2; }) == 2); + std::vector expected { 6, 5, 3, 4 }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 2") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v % 2 == 0; }) == 3); + std::vector expected { 1, 5, 3 }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 3") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 1 || v == 5; }) == 2); + std::vector expected { 6, 2, 3, 4 }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 4") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { (void)v; return true; }) == 6); + std::vector expected { }; + REQUIRE( vector == expected ); + } + SECTION("PopAll -- Test 1") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + const auto condition = [](int v) -> bool { return v == 1 || v == 2; }; + int poppedSum = 0; + const auto action = [&poppedSum](int v) { poppedSum += v; }; + REQUIRE(sfz::swapAndPopAll(vector, condition, action) == 2); + std::vector expected { 6, 5, 3, 4 }; + REQUIRE( vector == expected ); + REQUIRE( poppedSum == 3 ); + } + SECTION("PopAll -- Test 2") + { + std::vector vector { 1, 2, 3, 4, 5, 6 }; + const auto condition = [](int v) -> bool { return v % 2 == 0; }; + int poppedSum = 0; + const auto action = [&poppedSum](int v) { poppedSum += v; }; + REQUIRE(sfz::swapAndPopAll(vector, condition, action) == 3); + std::vector expected { 1, 5, 3 }; + REQUIRE( vector == expected ); + REQUIRE( poppedSum == 12 ); + } +}