FileCache refactoring and tests

libraries/networking/src/FileCache.cpp

@@ -11,52 +11,81 @@
 
 #include "FileCache.h"
 
-#include <cstdio>
-#include <cassert>
-#include <fstream>
-#include <unordered_set>
 
-#include <QDir>
+#include <unordered_set>
-#include <QSaveFile>
+#include <queue>
+#include <cassert>
+
+#include <QtCore/QDateTime>
+#include <QtCore/QDir>
+#include <QtCore/QSaveFile>
+#include <QtCore/QStorageInfo>
 
 #include <PathUtils.h>
+#include <NumericalConstants.h>
 
+#ifdef Q_OS_WIN
+#include <sys/utime.h>
+#else
+#include <utime.h>
+#endif
+
+#ifdef NDEBUG
 Q_LOGGING_CATEGORY(file_cache, "hifi.file_cache", QtWarningMsg)
-
+#else
+Q_LOGGING_CATEGORY(file_cache, "hifi.file_cache")
+#endif
 using namespace cache;
 
-static const std::string MANIFEST_NAME = "manifest";
+static const char DIR_SEP = '/';
+static const char EXT_SEP = '.';
 
-static const size_t BYTES_PER_MEGABYTES = 1024 * 1024;
+const size_t FileCache::DEFAULT_MAX_SIZE { GB_TO_BYTES(5) };
-static const size_t BYTES_PER_GIGABYTES = 1024 * BYTES_PER_MEGABYTES;
+const size_t FileCache::MAX_MAX_SIZE { GB_TO_BYTES(100) };
-const size_t FileCache::DEFAULT_UNUSED_MAX_SIZE = 5 * BYTES_PER_GIGABYTES; // 5GB
+const size_t FileCache::DEFAULT_MIN_FREE_STORAGE_SPACE { GB_TO_BYTES(1) };
-const size_t FileCache::MAX_UNUSED_MAX_SIZE = 100 * BYTES_PER_GIGABYTES; // 100GB
-const size_t FileCache::DEFAULT_OFFLINE_MAX_SIZE = 2 * BYTES_PER_GIGABYTES; // 2GB
 
-void FileCache::setUnusedFileCacheSize(size_t unusedFilesMaxSize) {
+
-    _unusedFilesMaxSize = std::min(unusedFilesMaxSize, MAX_UNUSED_MAX_SIZE);
+std::string getCacheName(const std::string& dirname_str) {
-    reserve(0);
+    QString dirname { dirname_str.c_str() };
+    QDir dir(dirname);
+    if (!dir.isAbsolute()) {
+        return dirname_str;
+    }
+    return dir.dirName().toStdString();
+}
+
+std::string getCachePath(const std::string& dirname_str) {
+    QString dirname { dirname_str.c_str() };
+    QDir dir(dirname);
+    if (dir.isAbsolute()) {
+        return dirname_str;
+    }
+    return PathUtils::getAppLocalDataFilePath(dirname).toStdString();
+}
+
+void FileCache::setMinFreeSize(size_t size) {
+    _minFreeSpaceSize = size;
+    clean();
     emit dirty();
 }
 
-void FileCache::setOfflineFileCacheSize(size_t offlineFilesMaxSize) {
+void FileCache::setMaxSize(size_t maxSize) {
-    _offlineFilesMaxSize = std::min(offlineFilesMaxSize, MAX_UNUSED_MAX_SIZE);
+    _maxSize = std::min(maxSize, MAX_MAX_SIZE);
+    clean();
+    emit dirty();
 }
 
 FileCache::FileCache(const std::string& dirname, const std::string& ext, QObject* parent) :
     QObject(parent),
     _ext(ext),
-    _dirname(dirname),
+    _dirname(getCacheName(dirname)),
-    _dirpath(PathUtils::getAppLocalDataFilePath(dirname.c_str()).toStdString()) {}
+    _dirpath(getCachePath(dirname)) {
+}
 
 FileCache::~FileCache() {
     clear();
 }
 
-void fileDeleter(File* file) {
-    file->deleter();
-}
-
 void FileCache::initialize() {
     QDir dir(_dirpath.c_str());
 
@@ -84,7 +113,7 @@ void FileCache::initialize() {
 }
 
 FilePointer FileCache::addFile(Metadata&& metadata, const std::string& filepath) {
-    FilePointer file(createFile(std::move(metadata), filepath).release(), &fileDeleter);
+    FilePointer file(createFile(std::move(metadata), filepath).release(), std::mem_fn(&File::deleter));
     if (file) {
         _numTotalFiles += 1;
         _totalFilesSize += file->getLength();
@@ -141,6 +170,7 @@ FilePointer FileCache::getFile(const Key& key) {
     if (it != _files.cend()) {
         file = it->second.lock();
         if (file) {
+            file->touch();
             // if it exists, it is active - remove it from the cache
             removeUnusedFile(file);
             qCDebug(file_cache, "[%s] Found %s", _dirname.c_str(), key.c_str());
@@ -155,44 +185,74 @@ FilePointer FileCache::getFile(const Key& key) {
 }
 
 std::string FileCache::getFilepath(const Key& key) {
-    return _dirpath + '/' + key + '.' + _ext;
+    return _dirpath + DIR_SEP + key + EXT_SEP + _ext;
 }
 
-void FileCache::addUnusedFile(const FilePointer file) {
+void FileCache::addUnusedFile(const FilePointer& file) {
     {
         Lock lock(_filesMutex);
         _files[file->getKey()] = file;
     }
 
-    reserve(file->getLength());
-    file->_LRUKey = ++_lastLRUKey;
-
     {
         Lock lock(_unusedFilesMutex);
-        _unusedFiles.insert({ file->_LRUKey, file });
+        _unusedFiles.insert(file);
         _numUnusedFiles += 1;
         _unusedFilesSize += file->getLength();
     }
+    clean();
 
     emit dirty();
 }
 
-void FileCache::removeUnusedFile(const FilePointer file) {
+void FileCache::removeUnusedFile(const FilePointer& file) {
     Lock lock(_unusedFilesMutex);
-    const auto it = _unusedFiles.find(file->_LRUKey);
+    if (_unusedFiles.erase(file)) {
-    if (it != _unusedFiles.cend()) {
-        _unusedFiles.erase(it);
         _numUnusedFiles -= 1;
         _unusedFilesSize -= file->getLength();
     }
 }
 
-void FileCache::reserve(size_t length) {
+size_t FileCache::getOverbudgetAmount() const {
+    size_t result = 0;
+
+    size_t currentFreeSpace = QStorageInfo(_dirpath.c_str()).bytesFree();
+    if (_minFreeSpaceSize > currentFreeSpace) {
+        result = _minFreeSpaceSize - currentFreeSpace;
+    }
+
+    if (_totalFilesSize > _maxSize) {
+        result = std::max(_totalFilesSize - _maxSize, result);
+    }
+
+    return result;
+}
+
+namespace cache {
+    struct FilePointerComparator {
+        bool operator()(const FilePointer& a, const FilePointer& b) {
+            return a->_modified > b->_modified;
+        }
+    };
+}
+
+void FileCache::clean() {
+    size_t overbudgetAmount = getOverbudgetAmount();
+
+    // Avoid sorting the unused files by LRU if we're not over budget / under free space
+    if (0 == overbudgetAmount) {
+        return;
+    }
+
     Lock unusedLock(_unusedFilesMutex);
-    while (!_unusedFiles.empty() &&
+    using Queue = std::priority_queue<FilePointer, std::vector<FilePointer>, FilePointerComparator>;
-            _unusedFilesSize + length > _unusedFilesMaxSize) {
+    Queue queue;
-        auto it = _unusedFiles.begin();
+    for (const auto& file : _unusedFiles) {
-        auto file = it->second;
+        queue.push(file);
+    }
+    while (!queue.empty() && overbudgetAmount > 0) {
+        auto file = queue.top();
+        queue.pop();
         auto length = file->getLength();
 
         unusedLock.unlock();
@@ -203,34 +263,32 @@ void FileCache::reserve(size_t length) {
         }
         unusedLock.lock();
 
-        _unusedFiles.erase(it);
+        _unusedFiles.erase(file);
         _numTotalFiles -= 1;
         _numUnusedFiles -= 1;
         _totalFilesSize -= length;
         _unusedFilesSize -= length;
+        overbudgetAmount -= std::min(length, overbudgetAmount);
     }
 }
 
 void FileCache::clear() {
-    Lock unusedFilesLock(_unusedFilesMutex);
+    // Eliminate any overbudget files
-    for (const auto& pair : _unusedFiles) {
+    clean();
-        auto& file = pair.second;
-        file->_cache = nullptr;
 
-        if (_totalFilesSize > _offlineFilesMaxSize) {
+    // Mark everything remaining as persisted
-            _totalFilesSize -= file->getLength();
+    Lock unusedFilesLock(_unusedFilesMutex);
-        } else {
+    for (auto& file : _unusedFiles) {
         file->_shouldPersist = true;
+        file->_cache = nullptr;
         qCDebug(file_cache, "[%s] Persisting %s", _dirname.c_str(), file->getKey().c_str());
     }
-    }
     _unusedFiles.clear();
 }
 
 void File::deleter() {
     if (_cache) {
-        FilePointer self(this, &fileDeleter);
+        _cache->addUnusedFile(FilePointer(this, std::mem_fn(&File::deleter)));
-        _cache->addUnusedFile(self);
     } else {
         deleteLater();
     }
@@ -239,7 +297,9 @@ void File::deleter() {
 File::File(Metadata&& metadata, const std::string& filepath) :
     _key(std::move(metadata.key)),
     _length(metadata.length),
-    _filepath(filepath) {}
+    _filepath(filepath),
+    _modified(QFileInfo(_filepath.c_str()).lastRead().toMSecsSinceEpoch()) {
+}
 
 File::~File() {
     QFile file(getFilepath().c_str());
@@ -248,3 +308,8 @@ File::~File() {
         file.remove();
     }
 }
+
+void File::touch() {
+    utime(_filepath.c_str(), nullptr);
+    _modified = std::max<int64_t>(QFileInfo(_filepath.c_str()).lastRead().toMSecsSinceEpoch(), _modified);
+}

libraries/networking/src/FileCache.h

@@ -14,6 +14,7 @@
 #include <memory>
 #include <cstddef>
 #include <map>
+#include <unordered_set>
 #include <mutex>
 #include <string>
 #include <unordered_map>
@@ -35,24 +36,28 @@ class FileCache : public QObject {
     Q_PROPERTY(size_t sizeTotal READ getSizeTotalFiles NOTIFY dirty)
     Q_PROPERTY(size_t sizeCached READ getSizeCachedFiles NOTIFY dirty)
 
-    static const size_t DEFAULT_UNUSED_MAX_SIZE;
+    static const size_t DEFAULT_MAX_SIZE;
-    static const size_t MAX_UNUSED_MAX_SIZE;
+    static const size_t MAX_MAX_SIZE;
-    static const size_t DEFAULT_OFFLINE_MAX_SIZE;
+    static const size_t DEFAULT_MIN_FREE_STORAGE_SPACE;
 
 public:
+    // You can initialize the FileCache with a directory name (ex.: "temp_jpgs") that will be relative to the application local data, OR with a full path
+    // The file cache will ignore any file that doesn't match the extension provided
+    FileCache(const std::string& dirname, const std::string& ext, QObject* parent = nullptr);
+    virtual ~FileCache();
+
     size_t getNumTotalFiles() const { return _numTotalFiles; }
     size_t getNumCachedFiles() const { return _numUnusedFiles; }
     size_t getSizeTotalFiles() const { return _totalFilesSize; }
     size_t getSizeCachedFiles() const { return _unusedFilesSize; }
 
-    void setUnusedFileCacheSize(size_t unusedFilesMaxSize);
+    // Set the maximum amount of disk space to use on disk
-    size_t getUnusedFileCacheSize() const { return _unusedFilesSize; }
+    void setMaxSize(size_t maxCacheSize);
 
-    void setOfflineFileCacheSize(size_t offlineFilesMaxSize);
+    // Set the minumum amount of free disk space to retain.  This supercedes the max size,
-
+    // so if the cache is consuming all but 500 MB of the drive, unused entries will be ejected 
-    // initialize FileCache with a directory name (not a path, ex.: "temp_jpgs") and an ext (ex.: "jpg")
+    // to free up more space, regardless of the cache max size
-    FileCache(const std::string& dirname, const std::string& ext, QObject* parent = nullptr);
+    void setMinFreeSize(size_t size);
-    virtual ~FileCache();
 
     using Key = std::string;
     struct Metadata {
@@ -76,10 +81,11 @@ public:
 signals:
     void dirty();
 
-protected:
+public:
     /// must be called after construction to create the cache on the fs and restore persisted files
     void initialize();
 
+    // Add file to the cache and return the cache entry.  
     FilePointer writeFile(const char* data, Metadata&& metadata, bool overwrite = false);
     FilePointer getFile(const Key& key);
 
@@ -95,11 +101,17 @@ private:
     std::string getFilepath(const Key& key);
 
     FilePointer addFile(Metadata&& metadata, const std::string& filepath);
-    void addUnusedFile(const FilePointer file);
+    void addUnusedFile(const FilePointer& file);
-    void removeUnusedFile(const FilePointer file);
+    void removeUnusedFile(const FilePointer& file);
-    void reserve(size_t length);
+    void clean();
     void clear();
 
+    size_t getOverbudgetAmount() const;
+
+    // FIXME it might be desirable to have the min free space variable be static so it can be
+    // shared among multiple instances of FileCache
+    std::atomic<size_t> _minFreeSpaceSize { DEFAULT_MIN_FREE_STORAGE_SPACE };
+    std::atomic<size_t> _maxSize { DEFAULT_MAX_SIZE };
     std::atomic<size_t> _numTotalFiles { 0 };
     std::atomic<size_t> _numUnusedFiles { 0 };
     std::atomic<size_t> _totalFilesSize { 0 };
@@ -113,12 +125,8 @@ private:
     std::unordered_map<Key, std::weak_ptr<File>> _files;
     Mutex _filesMutex;
 
-    std::map<int, FilePointer> _unusedFiles;
+    std::unordered_set<FilePointer> _unusedFiles;
     Mutex _unusedFilesMutex;
-    size_t _unusedFilesMaxSize { DEFAULT_UNUSED_MAX_SIZE };
-    int _lastLRUKey { 0 };
-
-    size_t _offlineFilesMaxSize { DEFAULT_OFFLINE_MAX_SIZE };
 };
 
 class File : public QObject {
@@ -142,13 +150,15 @@ protected:
 
 private:
     friend class FileCache;
+    friend struct FilePointerComparator;
 
     const Key _key;
     const size_t _length;
     const std::string _filepath;
 
-    FileCache* _cache;
+    void touch();
-    int _LRUKey { 0 };
+    FileCache* _cache { nullptr };
+    int64_t _modified { 0 };
 
     bool _shouldPersist { false };
 };

libraries/shared/src/NumericalConstants.h

@@ -50,10 +50,13 @@ const int KILO_PER_MEGA = 1000;
 
 #define KB_TO_BYTES_SHIFT 10
 #define MB_TO_BYTES_SHIFT 20
+#define GB_TO_BYTES_SHIFT 30
 
+#define GB_TO_BYTES(X) ((size_t)(X) << GB_TO_BYTES_SHIFT)
 #define MB_TO_BYTES(X) ((size_t)(X) << MB_TO_BYTES_SHIFT)
 #define KB_TO_BYTES(X) ((size_t)(X) << KB_TO_BYTES_SHIFT)
 
+#define BYTES_TO_GB(X) (X >> GB_TO_BYTES_SHIFT)
 #define BYTES_TO_MB(X) (X >> MB_TO_BYTES_SHIFT)
 #define BYTES_TO_KB(X) (X >> KB_TO_BYTES_SHIFT)
 

tests/networking/src/FileCacheTests.cpp

@@ -0,0 +1,170 @@
+//
+//  ResoruceTests.cpp
+//
+//  Copyright 2015 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+
+#include "FileCacheTests.h"
+
+#include <FileCache.h>
+
+QTEST_GUILESS_MAIN(FileCacheTests)
+
+using namespace cache;
+
+// Limit the file size to 10 MB
+static const size_t MAX_UNUSED_SIZE { 1024 * 1024 * 10 };
+static const QByteArray TEST_DATA { 1024 * 1024, '0' };
+
+static std::string getFileKey(int i) {
+    return QString(QByteArray { 1, (char)i }.toHex()).toStdString();
+}
+
+class TestFile : public File {
+    using Parent = File;
+public:
+    TestFile(Metadata&& metadata, const std::string& filepath)
+        : Parent(std::move(metadata), filepath) {
+    }
+};
+
+class TestFileCache : public FileCache {
+    using Parent = FileCache;
+
+public:
+    TestFileCache(const std::string& dirname, const std::string& ext, QObject* parent = nullptr) : Parent(dirname, ext, nullptr) {
+        initialize();
+    }
+
+    std::unique_ptr<File> createFile(Metadata&& metadata, const std::string& filepath) override {
+        qCInfo(file_cache) << "Wrote KTX" << metadata.key.c_str();
+        return std::unique_ptr<File>(new TestFile(std::move(metadata), filepath));
+    }
+};
+
+using CachePointer = std::shared_ptr<TestFileCache>;
+
+// The FileCache relies on deleteLater to clear unused files, but QTest classes don't run a conventional event loop
+// so we need to call this function to force any pending deletes to occur in the File destructor
+static void forceDeletes() {
+    while (QCoreApplication::hasPendingEvents()) {
+        QCoreApplication::sendPostedEvents();
+        QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
+        QCoreApplication::processEvents();
+    }
+}
+
+size_t FileCacheTests::getCacheDirectorySize() const {
+    size_t result = 0;
+    QDir dir(_testDir.path());
+    for (const auto& file : dir.entryList({ "*.tmp" })) {
+        result += QFileInfo(dir.absoluteFilePath(file)).size();
+    }
+    return result;
+}
+
+CachePointer makeFileCache(QString& location) {
+    auto result = std::make_shared<TestFileCache>(location.toStdString(), "tmp");
+    result->setMaxSize(MAX_UNUSED_SIZE);
+    return result;
+}
+
+void FileCacheTests::initTestCase() {
+}
+
+void FileCacheTests::testUnusedFiles() {
+    auto cache = makeFileCache(_testDir.path());
+    std::list<FilePointer> inUseFiles;
+
+    {
+        for (int i = 0; i < 100; ++i) {
+            std::string key = getFileKey(i);
+            auto file = cache->writeFile(TEST_DATA.data(), TestFileCache::Metadata(key, TEST_DATA.size()));
+            inUseFiles.push_back(file);
+            forceDeletes();
+            QThread::msleep(10);
+        }
+        QCOMPARE(cache->getNumCachedFiles(), (size_t)0);
+        QCOMPARE(cache->getNumTotalFiles(), (size_t)100);
+        // Release the in-use files
+        inUseFiles.clear();
+        // Cache state is updated, but the directory state is unchanged, 
+        // because the file deletes are triggered by an event loop
+        QCOMPARE(cache->getNumCachedFiles(), (size_t)10);
+        QCOMPARE(cache->getNumTotalFiles(), (size_t)10);
+        QVERIFY(getCacheDirectorySize() >  MAX_UNUSED_SIZE);
+        forceDeletes();
+        QCOMPARE(cache->getNumCachedFiles(), (size_t)10);
+        QCOMPARE(cache->getNumTotalFiles(), (size_t)10);
+        QVERIFY(getCacheDirectorySize() <= MAX_UNUSED_SIZE);
+    }
+
+    // Reset the cache
+    cache = makeFileCache(_testDir.path());
+    {
+        // Test files 0 to 89 are missing, because the LRU algorithm deleted them when we released the files
+        for (int i = 0; i < 90; ++i) {
+            std::string key = getFileKey(i);
+            auto file = cache->getFile(key);
+            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);
+        }
+        QCOMPARE(cache->getNumCachedFiles(), (size_t)0);
+        QCOMPARE(cache->getNumTotalFiles(), (size_t)10);
+        inUseFiles.clear();
+        QCOMPARE(cache->getNumCachedFiles(), (size_t)10);
+        QCOMPARE(cache->getNumTotalFiles(), (size_t)10);
+    }
+}
+
+size_t FileCacheTests::getFreeSpace() const {
+    return QStorageInfo(_testDir.path()).bytesFree();
+}
+
+// FIXME if something else is changing the amount of free space on the target drive concurrently with this test 
+// running, then it may fail
+void FileCacheTests::testFreeSpacePreservation() {
+    QCOMPARE(getCacheDirectorySize(), MAX_UNUSED_SIZE);
+    // Set the target free space to slightly above whatever the current free space is...
+    size_t targetFreeSpace = getFreeSpace() + MAX_UNUSED_SIZE / 2;
+
+    // Reset the cache
+    auto cache = makeFileCache(_testDir.path());
+    // Setting the min free space causes it to eject the oldest files that cause the cache to exceed the minimum space
+    cache->setMinFreeSize(targetFreeSpace);
+    QVERIFY(getFreeSpace() < targetFreeSpace);
+    forceDeletes();
+    QCOMPARE(cache->getNumCachedFiles(), (size_t)5);
+    QCOMPARE(cache->getNumTotalFiles(), (size_t)5);
+    QVERIFY(getFreeSpace() >= targetFreeSpace);
+    for (int i = 0; i < 95; ++i) {
+        std::string key = getFileKey(i);
+        auto file = cache->getFile(key);
+        QVERIFY(!file.get());
+    }
+    for (int i = 95; i < 100; ++i) {
+        std::string key = getFileKey(i);
+        auto file = cache->getFile(key);
+        QVERIFY(file.get());
+    }
+}
+
+void FileCacheTests::cleanupTestCase() {
+}
+

tests/networking/src/FileCacheTests.h

@@ -0,0 +1,30 @@
+//
+//  ResourceTests.h
+//
+//  Copyright 2015 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#ifndef hifi_ResourceTests_h
+#define hifi_ResourceTests_h
+
+#include <QtTest/QtTest>
+#include <QtCore/QTemporaryDir>
+
+class FileCacheTests : public QObject {
+    Q_OBJECT
+private slots:
+    void initTestCase();
+    void testUnusedFiles();
+    void testFreeSpacePreservation();
+    void cleanupTestCase();
+
+private:
+    size_t getFreeSpace() const;
+    size_t getCacheDirectorySize() const;
+    QTemporaryDir _testDir;
+};
+
+#endif // hifi_ResourceTests_h