From 6c70269fcca94eba51662250071c39ebe4342211 Mon Sep 17 00:00:00 2001 From: paulfd Date: Tue, 17 Sep 2019 20:09:26 +0200 Subject: [PATCH] Added a garbage collection to the file pool. Apparently something in the kernel or in libsndfile is memory-mapping the files, so that the memory usage stays quite high and there are no disk read. But since ASan does not complain, it seems okay... --- sfizz/FilePool.cpp | 45 ++++++++++++++++++++++++++++++++++++++------- sfizz/FilePool.h | 7 +++++++ sfizz/Voice.cpp | 6 ++++-- sfizz/Voice.h | 5 +++-- 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/sfizz/FilePool.cpp b/sfizz/FilePool.cpp index 71def3a4..918e01b3 100644 --- a/sfizz/FilePool.cpp +++ b/sfizz/FilePool.cpp @@ -28,7 +28,9 @@ #include "absl/types/span.h" #include #include +#include #include +#include using namespace std::chrono_literals; template @@ -42,7 +44,7 @@ std::unique_ptr> readFromFile(SndfileHandle& sndFile, int numFram sndFile.readf(tempReadBuffer->channelWriter(0), numFrames); ::readInterleaved(tempReadBuffer->getSpan(0), returnedBuffer->getSpan(0), returnedBuffer->getSpan(1)); } - return returnedBuffer; + return std::move(returnedBuffer); } std::optional sfz::FilePool::getFileInformation(std::string_view filename, uint32_t offset) noexcept @@ -79,11 +81,20 @@ std::optional sfz::FilePool::getFileInformation( if (preloadedData.contains(filename)) { auto alreadyPreloaded = preloadedData[filename]; if (preloadedSize > alreadyPreloaded->getNumFrames()) { + // FIXME: Okay, ideally here you would have a double indirection so that we can update _all_ the preloaded + // files in previous regions to account for the new offset + // + // Before this next command, some old regions and the file pool hold a shared pointer to the same audio buffer. + // This audio buffer is OK for the old regions, but too small for the new one. + // By resetting the filepool data to a new, longer audiobuffer, we are creating 2 copies of the same audio data. + // The filepool and the new regions have the longer copy, and the older regions have the shorter copy. + // This is not entirely optimal, but is it better to write a double shared pointer ? + // std::shared_ptr>> is a bit ugly... alreadyPreloaded.reset(readFromFile(sndFile, preloadedSize).release()); } returnedValue.preloadedData = alreadyPreloaded; } else { - returnedValue.preloadedData = std::shared_ptr>(readFromFile(sndFile, preloadedSize)); + returnedValue.preloadedData = readFromFile(sndFile, preloadedSize); preloadedData[filename] = returnedValue.preloadedData; } @@ -99,10 +110,11 @@ void sfz::FilePool::enqueueLoading(Voice* voice, std::string_view sample, int nu void sfz::FilePool::loadingThread() noexcept { - FileLoadingInformation fileToLoad {}; while (!quitThread) { - if (!loadingQueue.wait_dequeue_timed(fileToLoad, 10ms)) + FileLoadingInformation fileToLoad {}; + if (!loadingQueue.wait_dequeue_timed(fileToLoad, 200ms)) { continue; + } if (fileToLoad.voice == nullptr) { DBG("Background thread error: voice is null."); @@ -117,15 +129,34 @@ void sfz::FilePool::loadingThread() noexcept } SndfileHandle sndFile(reinterpret_cast(file.c_str())); - auto fileLoaded = std::make_unique>(sndFile.channels(), fileToLoad.numFrames); - fileToLoad.voice->setFileData(readFromFile(sndFile, fileToLoad.numFrames), fileToLoad.ticket); + + std::lock_guard guard { fileHandleMutex }; + auto newHandle = fileHandles.emplace_back(readFromFile(sndFile, fileToLoad.numFrames)); + fileToLoad.voice->setFileData(newHandle, fileToLoad.ticket); + } +} + +void sfz::FilePool::garbageThread() noexcept +{ + while (!quitThread) { + for (auto handle = fileHandles.begin(); handle < fileHandles.end();) { + if (handle->use_count() == 1) { + handle->reset(); + std::lock_guard guard { fileHandleMutex }; + std::iter_swap(handle, fileHandles.end() - 1); + fileHandles.pop_back(); + } else { + handle++; + } + } + std::this_thread::sleep_for(200ms); } } void sfz::FilePool::clear() { preloadedData.clear(); - while(loadingQueue.pop()){ + while (loadingQueue.pop()) { // Pop the queue } } \ No newline at end of file diff --git a/sfizz/FilePool.h b/sfizz/FilePool.h index 2da54176..c8041afa 100644 --- a/sfizz/FilePool.h +++ b/sfizz/FilePool.h @@ -30,6 +30,7 @@ #include "filesystem.h" #include "readerwriterqueue.h" #include +#include #include #include #include @@ -39,6 +40,7 @@ class FilePool { public: FilePool() : fileLoadingThread(std::thread(&FilePool::loadingThread, this)) + , garbageCollectionThread(std::thread(&FilePool::garbageThread, this)) { } @@ -46,6 +48,7 @@ public: { quitThread = true; fileLoadingThread.join(); + garbageCollectionThread.join(); } void setRootDirectory(const std::filesystem::path& directory) noexcept { rootDirectory = directory; } size_t getNumPreloadedSamples() const noexcept { return preloadedData.size(); } @@ -71,7 +74,11 @@ private: moodycamel::BlockingReaderWriterQueue loadingQueue { config::numVoices }; void loadingThread() noexcept; + void garbageThread() noexcept; std::thread fileLoadingThread; + std::thread garbageCollectionThread; + std::vector>> fileHandles; + std::mutex fileHandleMutex; bool quitThread { false }; absl::flat_hash_map>> preloadedData; LEAK_DETECTOR(FilePool); diff --git a/sfizz/Voice.cpp b/sfizz/Voice.cpp index f04f8ae1..5aef90ff 100644 --- a/sfizz/Voice.cpp +++ b/sfizz/Voice.cpp @@ -29,6 +29,7 @@ #include "SIMDHelpers.h" #include "SfzHelpers.h" #include "absl/algorithm/container.h" +#include sfz::Voice::Voice(const CCValueArray& ccState) : ccState(ccState) @@ -117,7 +118,7 @@ void sfz::Voice::prepareEGEnvelope(int delay, uint8_t velocity) noexcept normalizePercents(region->amplitudeEG.getStart(ccState, velocity))); } -void sfz::Voice::setFileData(std::unique_ptr> file, unsigned ticket) noexcept +void sfz::Voice::setFileData(std::shared_ptr> file, unsigned ticket) noexcept { if (ticket != this->ticket) return; @@ -473,13 +474,14 @@ sfz::Voice::TriggerType sfz::Voice::getTriggerType() const noexcept void sfz::Voice::reset() noexcept { dataReady.store(false); + fileData.reset(); state = State::idle; if (region != nullptr) { DBG("Reset voice with sample " << region->sample); } + region = nullptr; sourcePosition = 0; floatPositionOffset = 0.0f; - region = nullptr; noteIsOff = false; } diff --git a/sfizz/Voice.h b/sfizz/Voice.h index 42b18bcc..5ed9cd14 100644 --- a/sfizz/Voice.h +++ b/sfizz/Voice.h @@ -32,6 +32,7 @@ #include "LeakDetector.h" #include #include +#include namespace sfz { class Voice { @@ -49,7 +50,7 @@ public: void startVoice(Region* region, int delay, int channel, int number, uint8_t value, TriggerType triggerType) noexcept; void expectFileData(unsigned ticket); - void setFileData(std::unique_ptr> file, unsigned ticket) noexcept; + void setFileData(std::shared_ptr> file, unsigned ticket) noexcept; void registerNoteOff(int delay, int channel, int noteNumber, uint8_t velocity) noexcept; void registerCC(int delay, int channel, int ccNumber, uint8_t ccValue) noexcept; void registerPitchWheel(int delay, int channel, int pitch) noexcept; @@ -108,7 +109,7 @@ private: int initialDelay { 0 }; std::atomic dataReady { false }; - std::unique_ptr> fileData { nullptr }; + std::shared_ptr> fileData { nullptr }; unsigned ticket { 0 }; Buffer tempBuffer1;