From b9f9cddea0b3206fdb363a76160988c87bba5584 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 1 Jun 2017 16:40:16 -0700 Subject: [PATCH] Add versioning support to the KTX cache --- .../src/model-networking/KTXCache.cpp | 14 ++++++ .../src/model-networking/KTXCache.h | 5 +++ libraries/networking/src/FileCache.cpp | 45 +++++++++++++------ libraries/networking/src/FileCache.h | 16 +++++-- tests/networking/src/FileCacheTests.cpp | 27 ++++++++--- tests/networking/src/FileCacheTests.h | 1 + 6 files changed, 85 insertions(+), 23 deletions(-) diff --git a/libraries/model-networking/src/model-networking/KTXCache.cpp b/libraries/model-networking/src/model-networking/KTXCache.cpp index e0447af8e6..e6d6632571 100644 --- a/libraries/model-networking/src/model-networking/KTXCache.cpp +++ b/libraries/model-networking/src/model-networking/KTXCache.cpp @@ -11,14 +11,28 @@ #include "KTXCache.h" +#include #include using File = cache::File; using FilePointer = cache::FilePointer; +// Whenever a change is made to the serialized format for the KTX cache that isn't backward compatible, +// this value should be incremented. This will force the KTX cache to be wiped +const int KTXCache::CURRENT_VERSION = 0x01; +const int KTXCache::INVALID_VERSION = 0x00; + + KTXCache::KTXCache(const std::string& dir, const std::string& ext) : FileCache(dir, ext) { initialize(); + + Setting::Handle cacheVersionHandle("hifi.ktx.cache_version", KTXCache::INVALID_VERSION); + auto cacheVersion = cacheVersionHandle.get(); + if (cacheVersion != CURRENT_VERSION) { + wipe(); + cacheVersionHandle.set(CURRENT_VERSION); + } } KTXFilePointer KTXCache::writeFile(const char* data, Metadata&& metadata) { diff --git a/libraries/model-networking/src/model-networking/KTXCache.h b/libraries/model-networking/src/model-networking/KTXCache.h index bbf7ceadea..2a3f191b53 100644 --- a/libraries/model-networking/src/model-networking/KTXCache.h +++ b/libraries/model-networking/src/model-networking/KTXCache.h @@ -27,6 +27,11 @@ class KTXCache : public cache::FileCache { Q_OBJECT public: + // Whenever a change is made to the serialized format for the KTX cache that isn't backward compatible, + // this value should be incremented. This will force the KTX cache to be wiped + static const int CURRENT_VERSION; + static const int INVALID_VERSION; + KTXCache(const std::string& dir, const std::string& ext); KTXFilePointer writeFile(const char* data, Metadata&& metadata); diff --git a/libraries/networking/src/FileCache.cpp b/libraries/networking/src/FileCache.cpp index 6cb8cd8f7c..95304e3866 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/networking/src/FileCache.cpp @@ -236,6 +236,28 @@ namespace cache { }; } +void FileCache::eject(const FilePointer& file) { + file->_cache = nullptr; + const auto& length = file->getLength(); + const auto& key = file->getKey(); + + { + Lock lock(_filesMutex); + if (0 != _files.erase(key)) { + _numTotalFiles -= 1; + _totalFilesSize -= length; + } + } + + { + Lock unusedLock(_unusedFilesMutex); + if (0 != _unusedFiles.erase(file)) { + _numUnusedFiles -= 1; + _unusedFilesSize -= length; + } + } +} + void FileCache::clean() { size_t overbudgetAmount = getOverbudgetAmount(); @@ -250,28 +272,23 @@ void FileCache::clean() { for (const auto& file : _unusedFiles) { queue.push(file); } + while (!queue.empty() && overbudgetAmount > 0) { auto file = queue.top(); queue.pop(); + eject(file); auto length = file->getLength(); - - unusedLock.unlock(); - { - file->_cache = nullptr; - Lock lock(_filesMutex); - _files.erase(file->getKey()); - } - unusedLock.lock(); - - _unusedFiles.erase(file); - _numTotalFiles -= 1; - _numUnusedFiles -= 1; - _totalFilesSize -= length; - _unusedFilesSize -= length; overbudgetAmount -= std::min(length, overbudgetAmount); } } +void FileCache::wipe() { + Lock unusedFilesLock(_unusedFilesMutex); + while (!_unusedFiles.empty()) { + eject(*_unusedFiles.begin()); + } +} + void FileCache::clear() { // Eliminate any overbudget files clean(); diff --git a/libraries/networking/src/FileCache.h b/libraries/networking/src/FileCache.h index 040e1ab592..f29d75f779 100644 --- a/libraries/networking/src/FileCache.h +++ b/libraries/networking/src/FileCache.h @@ -46,6 +46,9 @@ public: FileCache(const std::string& dirname, const std::string& ext, QObject* parent = nullptr); virtual ~FileCache(); + // Remove all unlocked items from the cache + void wipe(); + size_t getNumTotalFiles() const { return _numTotalFiles; } size_t getNumCachedFiles() const { return _numUnusedFiles; } size_t getSizeTotalFiles() const { return _totalFilesSize; } @@ -95,6 +98,9 @@ public: private: using Mutex = std::recursive_mutex; using Lock = std::unique_lock; + using Map = std::unordered_map>; + using Set = std::unordered_set; + using KeySet = std::unordered_set; friend class File; @@ -105,6 +111,8 @@ private: void removeUnusedFile(const FilePointer& file); void clean(); void clear(); + // Remove a file from the cache + void eject(const FilePointer& file); size_t getOverbudgetAmount() const; @@ -122,10 +130,10 @@ private: std::string _dirpath; bool _initialized { false }; - std::unordered_map> _files; + Map _files; Mutex _filesMutex; - std::unordered_set _unusedFiles; + Set _unusedFiles; Mutex _unusedFilesMutex; }; @@ -136,8 +144,8 @@ public: using Key = FileCache::Key; using Metadata = FileCache::Metadata; - Key getKey() const { return _key; } - size_t getLength() const { return _length; } + const Key& getKey() const { return _key; } + const size_t& getLength() const { return _length; } std::string getFilepath() const { return _filepath; } virtual ~File(); diff --git a/tests/networking/src/FileCacheTests.cpp b/tests/networking/src/FileCacheTests.cpp index 0813d05a54..79fe9dee54 100644 --- a/tests/networking/src/FileCacheTests.cpp +++ b/tests/networking/src/FileCacheTests.cpp @@ -113,18 +113,21 @@ void FileCacheTests::testUnusedFiles() { QVERIFY(!file.get()); } - QThread::msleep(1000); // Test files 90 to 99 are present for (int i = 90; i < 100; ++i) { std::string key = getFileKey(i); auto file = cache->getFile(key); QVERIFY(file.get()); inUseFiles.push_back(file); - // Each access touches the file, so we need to sleep here to ensure that the files are - // spaced out in numeric order, otherwise later tests can't reliably determine the order - // for cache ejection - QThread::msleep(1000); + + if (i == 94) { + // Each access touches the file, so we need to sleep here to ensure that the the last 5 files + // have later times for cache ejection priority, otherwise the test runs too fast to reliably + // differentiate + QThread::msleep(1000); + } } + QCOMPARE(cache->getNumCachedFiles(), (size_t)0); QCOMPARE(cache->getNumTotalFiles(), (size_t)10); inUseFiles.clear(); @@ -165,6 +168,20 @@ void FileCacheTests::testFreeSpacePreservation() { } } +void FileCacheTests::testWipe() { + // Reset the cache + auto cache = makeFileCache(_testDir.path()); + QCOMPARE(cache->getNumCachedFiles(), (size_t)5); + QCOMPARE(cache->getNumTotalFiles(), (size_t)5); + cache->wipe(); + QCOMPARE(cache->getNumCachedFiles(), (size_t)0); + QCOMPARE(cache->getNumTotalFiles(), (size_t)0); + QVERIFY(getCacheDirectorySize() > 0); + forceDeletes(); + QCOMPARE(getCacheDirectorySize(), (size_t)0); +} + + void FileCacheTests::cleanupTestCase() { } diff --git a/tests/networking/src/FileCacheTests.h b/tests/networking/src/FileCacheTests.h index 838c15afb8..b34b384855 100644 --- a/tests/networking/src/FileCacheTests.h +++ b/tests/networking/src/FileCacheTests.h @@ -20,6 +20,7 @@ private slots: void testUnusedFiles(); void testFreeSpacePreservation(); void cleanupTestCase(); + void testWipe(); private: size_t getFreeSpace() const;