Merge pull request #276 from paulfd/swap-and-pop

Implement helpers for the swap and pop idioms

src/sfizz/EQPool.cpp

@@ -2,6 +2,7 @@
 #include <thread>
 #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<std::mutex> 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<EQHolder>(midiState));
         eqs.back()->setSampleRate(sampleRate);

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

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 {};

src/sfizz/FilterPool.cpp

@@ -1,5 +1,6 @@
 #include "FilterPool.h"
 #include "SIMDHelpers.h"
+#include "SwapAndPop.h"
 #include "absl/algorithm/container.h"
 #include <thread>
 #include <chrono>
@@ -134,18 +135,8 @@ size_t sfz::FilterPool::setNumFilters(size_t numFilters)
 {
     const std::lock_guard<std::mutex> 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<FilterHolder>(midiState));
         filters.back()->setSampleRate(sampleRate);

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<class T, class F, class A>
+inline unsigned swapAndPopAll(std::vector<T>& 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<class T, class F>
+inline unsigned swapAndPopAll(std::vector<T>& vector, F&& condition)
+{
+    return swapAndPopAll(vector, std::forward<F>(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<class T, class F, class A>
+inline bool swapAndPopFirst(std::vector<T>& 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<class T, class F>
+inline unsigned swapAndPopFirst(std::vector<T>& vector, F&& condition)
+{
+    return swapAndPopFirst(vector, std::forward<F>(condition), [](T& v){ (void)v; });
+}
+
+}

tests/CMakeLists.txt

@@ -31,6 +31,7 @@ set(SFIZZ_TEST_SOURCES
     FloatHelpersT.cpp
     WavetablesT.cpp
     SemaphoreT.cpp
+    SwapAndPopT.cpp
     TuningT.cpp
 )
 

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<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 2; }));
+        std::vector<int> expected { 1, 6, 3, 4, 5 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopFirst -- Test 2")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 5; }));
+        std::vector<int> expected { 1, 2, 3, 4, 6 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopFirst -- Test 3")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 1; }));
+        std::vector<int> expected { 6, 2, 3, 4, 5 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopFirst -- Test 4")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopFirst(vector, [](int v) -> bool { return v == 6; }));
+        std::vector<int> expected { 1, 2, 3, 4, 5 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 1")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 2; }) == 1);
+        std::vector<int> expected { 1, 6, 3, 4, 5 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 2")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 5; }) == 1);
+        std::vector<int> expected { 1, 2, 3, 4, 6 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 3")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 1; }) == 1);
+        std::vector<int> expected { 6, 2, 3, 4, 5 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 4")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 6; }) == 1);
+        std::vector<int> expected { 1, 2, 3, 4, 5 };
+        REQUIRE( vector == expected );
+    }
+}
+
+TEST_CASE("[SwapAndPop] Popping multiple elements")
+{
+    SECTION("PopAll -- Test 1")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 1 || v == 2; }) == 2);
+        std::vector<int> expected { 6, 5, 3, 4 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 2")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v % 2 == 0; }) == 3);
+        std::vector<int> expected { 1, 5, 3 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 3")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { return v == 1 || v == 5; }) == 2);
+        std::vector<int> expected { 6, 2, 3, 4 };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 4")
+    {
+        std::vector<int> vector { 1, 2, 3, 4, 5, 6 };
+        REQUIRE(sfz::swapAndPopAll(vector, [](int v) -> bool { (void)v; return true; }) == 6);
+        std::vector<int> expected { };
+        REQUIRE( vector == expected );
+    }
+    SECTION("PopAll -- Test 1")
+    {
+        std::vector<int> 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<int> expected { 6, 5, 3, 4 };
+        REQUIRE( vector == expected );
+        REQUIRE( poppedSum == 3 );
+    }
+    SECTION("PopAll -- Test 2")
+    {
+        std::vector<int> 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<int> expected { 1, 5, 3 };
+        REQUIRE( vector == expected );
+        REQUIRE( poppedSum == 12 );
+    }
+}