diff --git a/src/core/fullscreenui_settings.cpp b/src/core/fullscreenui_settings.cpp index 5ba2e0bf6..3d6f9c1cb 100644 --- a/src/core/fullscreenui_settings.cpp +++ b/src/core/fullscreenui_settings.cpp @@ -5773,13 +5773,13 @@ void FullscreenUI::DrawAchievementsSettingsPage(std::unique_lock& se MenuButtonWithoutSummary(str, false); } - if (const auto cache = HTTPCache::GetCacheArchive(); cache->IsOpen()) + if (const ObjectArchive& cache = HTTPCache::GetCacheArchive(); cache.IsOpen()) { static constexpr auto to_mb = [](s64 size) { return static_cast((size + 1048575) / 1048576); }; - const u64 size = cache->GetTotalSize(); - const u64 object_size = cache->GetTotalObjectSize(); - const size_t count = cache->GetSize(); + const u64 size = cache.GetTotalSize(); + const u64 object_size = cache.GetTotalObjectSize(); + const size_t count = cache.GetSize(); str.format(fmt::runtime(FSUI_ICONVSTR(ICON_FA_GLOBE, "Web Cache Size: {0} MB ({1} MB in {2} objects)")), to_mb(size), to_mb(object_size), count); diff --git a/src/duckstation-qt/advancedsettingswidget.cpp b/src/duckstation-qt/advancedsettingswidget.cpp index d711e3952..e3249d283 100644 --- a/src/duckstation-qt/advancedsettingswidget.cpp +++ b/src/duckstation-qt/advancedsettingswidget.cpp @@ -133,12 +133,12 @@ void AdvancedSettingsWidget::onShowDebugOptionsStateChanged() void AdvancedSettingsWidget::refreshWebCacheSize() { - const auto cache = HTTPCache::GetCacheArchive(); + const ObjectArchive& cache = HTTPCache::GetCacheArchive(); static constexpr auto to_mb = [](s64 size) { return static_cast((size + 1048575) / 1048576); }; - const u64 cache_size = cache->GetTotalSize(); - const u64 object_size = cache->GetTotalObjectSize(); - const size_t num_objects = cache->GetSize(); + const u64 cache_size = cache.GetTotalSize(); + const u64 object_size = cache.GetTotalObjectSize(); + const size_t num_objects = cache.GetSize(); m_ui.webCacheSize->setText(tr("Current Cache Size: %1 MB (%2 MB in %3 objects)") .arg(to_mb(cache_size)) diff --git a/src/duckstation-qt/asyncpixmaploader.cpp b/src/duckstation-qt/asyncpixmaploader.cpp index edd8b352c..364daaadd 100644 --- a/src/duckstation-qt/asyncpixmaploader.cpp +++ b/src/duckstation-qt/asyncpixmaploader.cpp @@ -31,8 +31,8 @@ bool AsyncPixmapLoader::isQueueNeeded(std::string_view url_or_path) return false; // Don't try to async load when we don't have cache. - const auto cache = HTTPCache::GetCacheArchive(); - return (cache->IsOpen() && !cache->Contains(HTTPCache::URLToCacheKey(url_or_path))); + const ObjectArchive& cache = HTTPCache::GetCacheArchive(); + return (cache.IsOpen() && !cache.Contains(HTTPCache::URLToCacheKey(url_or_path))); } static std::string_view GetExtensionFromURL(std::string_view url) diff --git a/src/util/http_cache.cpp b/src/util/http_cache.cpp index 4e199b388..10f4ac34d 100644 --- a/src/util/http_cache.cpp +++ b/src/util/http_cache.cpp @@ -12,11 +12,13 @@ #include "common/log.h" #include "common/path.h" #include "common/string_util.h" +#include "common/thirdparty/SmallVector.h" #include #include #include #include +#include #include @@ -28,7 +30,8 @@ namespace HTTPCache { static constexpr u32 CACHE_VERSION = 1; -static bool QueueDownload(std::string_view url, FetchCallback callback, Error* error); +static void QueueDownload(std::string_view url, FetchCallback callback, Error* error, + std::unique_lock&& lock); static void DownloadCallback(const std::string& url, s32 status_code, const Error& error, const std::string& content_type, const HTTPDownloader::RequestData& data); @@ -38,8 +41,8 @@ struct Locals { ObjectArchive cache_archive; std::deque> pending_downloads; - std::mutex cache_mutex; - bool tried_initialize_cache_archive = false; + std::mutex pending_downloads_lock; + std::once_flag cache_open_flag; }; } // namespace @@ -77,16 +80,17 @@ void HTTPCache::Shutdown() // awkward situation where a request callback could create another downloader... HTTPDownloader::CancelRequestsForOwner(&s_locals); - const std::unique_lock cache_lock(s_locals.cache_mutex); - for (auto iter = s_locals.pending_downloads.begin(); iter != s_locals.pending_downloads.end();) { - if (iter->second) - iter->second({}); - iter = s_locals.pending_downloads.erase(iter); + const std::unique_lock cache_lock(s_locals.pending_downloads_lock); + for (auto iter = s_locals.pending_downloads.begin(); iter != s_locals.pending_downloads.end();) + { + if (iter->second) + iter->second({}); + iter = s_locals.pending_downloads.erase(iter); + } } - if (s_locals.cache_archive.IsOpen()) - s_locals.cache_archive.Close(); + s_locals.cache_archive.Close(); } std::span HTTPCache::URLToCacheKey(std::string_view key) @@ -94,31 +98,26 @@ std::span HTTPCache::URLToCacheKey(std::string_view key) return std::span(reinterpret_cast(key.data()), key.size()); } -HTTPCache::CacheArchivePtr HTTPCache::GetCacheArchive() +ObjectArchive& HTTPCache::GetCacheArchive() { - std::unique_lock lock(s_locals.cache_mutex); + // Opens once, never closes. Therefore this is safe to skip the once_flag in the fast path. if (!s_locals.cache_archive.IsOpen()) [[unlikely]] { - if (!s_locals.tried_initialize_cache_archive) - { - s_locals.tried_initialize_cache_archive = true; - + std::call_once(s_locals.cache_open_flag, []() { Error error; std::string cache_path = Path::Combine(EmuFolders::Cache, "http_cache"); if (!s_locals.cache_archive.OpenPath(cache_path, CACHE_VERSION, &error)) ERROR_LOG("Failed to initialize HTTP cache: {}", error.GetDescription()); - } + }); } - return CacheArchivePtr(std::move(lock), &s_locals.cache_archive); + return s_locals.cache_archive; } HTTPCache::LookupResult HTTPCache::Lookup(std::string_view url, Error* error) { - const auto cache = GetCacheArchive(); - Error lookup_error; - std::optional image_data = cache->Lookup(URLToCacheKey(url), &lookup_error); + std::optional image_data = GetCacheArchive().Lookup(URLToCacheKey(url), &lookup_error); if (image_data.has_value()) { return LookupResult(LookupStatus::Hit, std::move(*image_data)); @@ -141,13 +140,17 @@ HTTPCache::LookupResult HTTPCache::LookupOrFetch(std::string_view url, Error* er { std::optional image_data; - const auto cache = GetCacheArchive(); - Error lookup_error; - image_data = cache->Lookup(URLToCacheKey(url), &lookup_error); - if (!image_data.has_value() && lookup_error.GetDescription() != ObjectArchive::ERROR_DESCRIPTION_DOES_NOT_EXIST) - [[unlikely]] + image_data = GetCacheArchive().Lookup(URLToCacheKey(url), &lookup_error); + + // did we find it? return the data directly without invoking the callback + if (image_data.has_value()) + { + return LookupResult(LookupStatus::Hit, std::move(*image_data)); + } + else if (lookup_error.GetDescription() != ObjectArchive::ERROR_DESCRIPTION_DOES_NOT_EXIST) [[unlikely]] { + // Errors are unrecoverable. ERROR_LOG("Failed to read cached texture data for URL '{}': {}", url, lookup_error.GetDescription()); if (error) *error = std::move(lookup_error); @@ -155,18 +158,29 @@ HTTPCache::LookupResult HTTPCache::LookupOrFetch(std::string_view url, Error* er return LookupResult(LookupStatus::Error); } - // did we find it? return the data directly without invoking the callback + // Try the lookup again with the lock held, core thread could have completed in the meantime. + std::unique_lock lock(s_locals.pending_downloads_lock); + image_data = GetCacheArchive().Lookup(URLToCacheKey(url), &lookup_error); if (image_data.has_value()) + { return LookupResult(LookupStatus::Hit, std::move(*image_data)); + } + else if (lookup_error.GetDescription() != ObjectArchive::ERROR_DESCRIPTION_DOES_NOT_EXIST) [[unlikely]] + { + ERROR_LOG("Failed to read cached texture data for URL '{}': {}", url, lookup_error.GetDescription()); + if (error) + *error = std::move(lookup_error); + + return LookupResult(LookupStatus::Error); + } - return QueueDownload(url, std::move(callback), error) ? LookupResult(LookupStatus::Miss) : - LookupResult(LookupStatus::Error); + QueueDownload(url, std::move(callback), error, std::move(lock)); + return LookupResult(LookupStatus::Miss); } -bool HTTPCache::QueueDownload(std::string_view url, FetchCallback callback, Error* error) +void HTTPCache::QueueDownload(std::string_view url, FetchCallback callback, Error* error, + std::unique_lock&& lock) { - // NOTE: Assumes that the lock is held - // do we already have a request? const bool has_request = std::ranges::any_of(s_locals.pending_downloads, [url](const auto& pair) { return pair.first == url; }); @@ -178,30 +192,45 @@ bool HTTPCache::QueueDownload(std::string_view url, FetchCallback callback, Erro // don't queue it twice if (has_request) - return true; + return; DEV_LOG("Cache miss for URL '{}', downloading...", url); + // release lock because CreateRequest() can fire the callback immediately + lock.unlock(); HTTPDownloader::CreateRequest(std::string(url), &s_locals, [url = std::string(url)](s32 status_code, Error& error, std::string& content_type, HTTPDownloader::RequestData& data) { DownloadCallback(url, status_code, error, content_type, std::move(data)); }); - - return true; } void HTTPCache::DownloadCallback(const std::string& url, s32 status_code, const Error& error, const std::string& content_type, const HTTPDownloader::RequestData& data) { + // hold the lock for the insertion, so we don't create a duplicate request as described in Lookup() + std::unique_lock lock(s_locals.pending_downloads_lock); + + // don't insert into cache on failure const bool success = (status_code == HTTPDownloader::HTTP_STATUS_OK); - if (!success) - ERROR_LOG("Failed to download '{}': HTTP status code {}, error: {}", url, status_code, error.GetDescription()); + if (success) + { + VERBOSE_LOG("Adding URL '{}' to cache ({} bytes)", url, data.size()); - const auto cache = GetCacheArchive(); - DebugAssert(cache); + // TODO: only compress if it's not images + Error insert_error; + if (!GetCacheArchive().Insert(URLToCacheKey(url), data, ObjectArchive::CompressType::Uncompressed, &insert_error)) + { + if (insert_error.GetDescription() != ObjectArchive::ERROR_DESCRIPTION_ALREADY_EXISTS) + ERROR_LOG("Failed to insert downloaded data for URL '{}' into cache: {}", url, insert_error.GetDescription()); + } + } + else + { + ERROR_LOG("Failed to download '{}': HTTP status code {}, error: {}", url, status_code, error.GetDescription()); + } - // invoke all callbacks + // invoke all callbacks. uses indexing in case something gets added in the callback for (auto iter = s_locals.pending_downloads.begin(); iter != s_locals.pending_downloads.end();) { if (iter->first != url) @@ -211,71 +240,108 @@ void HTTPCache::DownloadCallback(const std::string& url, s32 status_code, const } if (iter->second) - iter->second(success ? data : std::span()); - - iter = s_locals.pending_downloads.erase(iter); - } - - // don't insert into cache on failure - if (!success) - return; - - // NOTE: we're not doing this on a worker thread because if we queue it, another request can come in - // for the same url, which will re-trigger a download... - VERBOSE_LOG("Adding URL '{}' to cache ({} bytes)", url, data.size()); - - // TODO: only compress if it's images - Error insert_error; - if (!cache->Insert(URLToCacheKey(url), data, ObjectArchive::CompressType::Uncompressed, &insert_error)) - { - if (insert_error.GetDescription() != ObjectArchive::ERROR_DESCRIPTION_ALREADY_EXISTS) - ERROR_LOG("Failed to insert downloaded data for URL '{}' into cache: {}", url, insert_error.GetDescription()); + { + // callback could queue another download and invalidate the iterator, so shove all callbacks for + // the same url into a temporary list before executing them. + llvm::SmallVector pending_callbacks; + for (; iter != s_locals.pending_downloads.end();) + { + if (iter->first == url) + { + if (iter->second) + pending_callbacks.push_back(std::move(iter->second)); + + iter = s_locals.pending_downloads.erase(iter); + } + else + { + ++iter; + } + } + + lock.unlock(); + for (FetchCallback& callback : pending_callbacks) + callback(success ? data : std::span()); + + // all requests for this url have been processed, so we don't need to do anything else here + return; + } + else + { + iter = s_locals.pending_downloads.erase(iter); + } } } bool HTTPCache::Contains(std::string_view url) { - return GetCacheArchive()->Contains(URLToCacheKey(url)); + return GetCacheArchive().Contains(URLToCacheKey(url)); } void HTTPCache::Prefetch(std::string_view url) { // skip early if already cached, or cannot prefetch - const auto cache = GetCacheArchive(); - if (!cache->IsOpen() || cache->Contains(URLToCacheKey(url))) [[unlikely]] + const ObjectArchive& cache = GetCacheArchive(); + if (!cache.IsOpen() || cache.Contains(URLToCacheKey(url))) [[unlikely]] return; // queue a download with no callback, which will cause it to be cached when it completes - QueueDownload(url, {}, nullptr); + std::unique_lock lock(s_locals.pending_downloads_lock); + + // see Lookup() for why we check again. + if (cache.Contains(URLToCacheKey(url))) + return; + + QueueDownload(url, {}, nullptr, std::move(lock)); } void HTTPCache::Prefetch(std::string_view url, PrefetchCallback callback) { - const auto cache = GetCacheArchive(); + const ObjectArchive& cache = GetCacheArchive(); - if (!cache->IsOpen()) [[unlikely]] + if (!cache.IsOpen()) [[unlikely]] { callback(false); return; } // skip early if already cached - if (cache->Contains(URLToCacheKey(url))) + if (cache.Contains(URLToCacheKey(url))) { callback(true); return; } // queue a download with no callback, which will cause it to be cached when it completes - QueueDownload(url, [callback = std::move(callback)](std::span data) { callback(!data.empty()); }, nullptr); + std::unique_lock lock(s_locals.pending_downloads_lock); + + // see Lookup() for why we check again. + if (cache.Contains(URLToCacheKey(url))) + { + lock.unlock(); + callback(true); + return; + } + + QueueDownload( + url, [callback = std::move(callback)](std::span data) { callback(!data.empty()); }, nullptr, + std::move(lock)); } void HTTPCache::WaitForAllPrefetchRequests() { - HTTPDownloader::WaitForAllRequestsFromOwner(&s_locals); + // there's a small window before the request has been created, handle it by checking against the pending list + for (;;) + { + HTTPDownloader::WaitForAllRequestsFromOwner(&s_locals); + + std::unique_lock lock(s_locals.pending_downloads_lock); + if (s_locals.pending_downloads.empty()) + break; + } } bool HTTPCache::Clear(Error* error) { - return GetCacheArchive()->Clear(error); + return GetCacheArchive().Clear(error); } diff --git a/src/util/http_cache.h b/src/util/http_cache.h index 81efa11bc..0ec5445de 100644 --- a/src/util/http_cache.h +++ b/src/util/http_cache.h @@ -4,12 +4,10 @@ #pragma once #include "common/heap_array.h" -#include "common/locked_ptr.h" #include "common/optional_with_status.h" #include "common/types.h" #include -#include #include class Error; @@ -17,9 +15,6 @@ class ObjectArchive; namespace HTTPCache { -/// Thread-safe locked pointer to the shared cache archive. Holds the cache mutex for its lifetime. -using CacheArchivePtr = LockedPtr; - /// Data returned from a successful lookup. using LookupData = DynamicHeapArray; @@ -52,8 +47,8 @@ void Shutdown(); /// Converts a URL to a cache key. std::span URLToCacheKey(std::string_view key); -/// Returns a locked pointer to the shared cache archive, opening it on first use. -CacheArchivePtr GetCacheArchive(); +/// Returns a pointer to the shared cache archive, opening it on first use. +ObjectArchive& GetCacheArchive(); /// Looks up @p url in the cache. /// On a Hit, the returned LookupResult holds the cached data.