From 168d177197bd7558bbe0ca13d01c984ad8638da7 Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 9 Mar 2021 14:39:51 -0800 Subject: This set of changes reverts the merge with master (git revert c83e740) and results in a version of the DRTVWR-519 that matches what was presemt before it was deployed as a release viewer *plus* 3 small fixes from Maxim (See commits). This branch can now be used for additional fixes before eventually being used to release D-519 as normal --- indra/llfilesystem/lldiskcache.cpp | 327 +++++++++++++++++++++++++++++++++++++ 1 file changed, 327 insertions(+) create mode 100644 indra/llfilesystem/lldiskcache.cpp (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp new file mode 100644 index 0000000000..c9f7684b5a --- /dev/null +++ b/indra/llfilesystem/lldiskcache.cpp @@ -0,0 +1,327 @@ +/** + * @file lldiskcache.cpp + * @brief The disk cache implementation. + * + * Note: Rather than keep the top level function comments up + * to date in both the source and header files, I elected to + * only have explicit comments about each function and variable + * in the header - look there for details. The same is true for + * description of how this code is supposed to work. + * + * $LicenseInfo:firstyear=2009&license=viewerlgpl$ + * Second Life Viewer Source Code + * Copyright (C) 2020, Linden Research, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" +#include "llassettype.h" +#include "lldir.h" +#include +#include +#include + +#include "lldiskcache.h" + +LLDiskCache::LLDiskCache(const std::string cache_dir, + const int max_size_bytes, + const bool enable_cache_debug_info) : + mCacheDir(cache_dir), + mMaxSizeBytes(max_size_bytes), + mEnableCacheDebugInfo(enable_cache_debug_info) +{ + mCacheFilenamePrefix = "sl_cache"; + + LLFile::mkdir(cache_dir); +} + +void LLDiskCache::purge() +{ + if (mEnableCacheDebugInfo) + { + LL_INFOS() << "Total dir size before purge is " << dirFileSize(mCacheDir) << LL_ENDL; + } + + auto start_time = std::chrono::high_resolution_clock::now(); + + typedef std::pair> file_info_t; + std::vector file_info; + +#if LL_WINDOWS + std::wstring cache_path(utf8str_to_utf16str(mCacheDir)); +#else + std::string cache_path(mCacheDir); +#endif + if (boost::filesystem::is_directory(cache_path)) + { + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + { + if (boost::filesystem::is_regular_file(entry)) + { + if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) + { + uintmax_t file_size = boost::filesystem::file_size(entry); + const std::string file_path = entry.path().string(); + const std::time_t file_time = boost::filesystem::last_write_time(entry); + + file_info.push_back(file_info_t(file_time, { file_size, file_path })); + } + } + } + } + + std::sort(file_info.begin(), file_info.end(), [](file_info_t& x, file_info_t& y) + { + return x.first > y.first; + }); + + LL_INFOS() << "Purging cache to a maximum of " << mMaxSizeBytes << " bytes" << LL_ENDL; + + uintmax_t file_size_total = 0; + for (file_info_t& entry : file_info) + { + file_size_total += entry.second.first; + + std::string action = ""; + if (file_size_total > mMaxSizeBytes) + { + action = "DELETE:"; + boost::filesystem::remove(entry.second.second); + } + else + { + action = " KEEP:"; + } + + if (mEnableCacheDebugInfo) + { + // have to do this because of LL_INFO/LL_END weirdness + std::ostringstream line; + + line << action << " "; + line << entry.first << " "; + line << entry.second.first << " "; + line << entry.second.second; + line << " (" << file_size_total << "/" << mMaxSizeBytes << ")"; + LL_INFOS() << line.str() << LL_ENDL; + } + } + + if (mEnableCacheDebugInfo) + { + auto end_time = std::chrono::high_resolution_clock::now(); + auto execute_time = std::chrono::duration_cast(end_time - start_time).count(); + LL_INFOS() << "Total dir size after purge is " << dirFileSize(mCacheDir) << LL_ENDL; + LL_INFOS() << "Cache purge took " << execute_time << " ms to execute for " << file_info.size() << " files" << LL_ENDL; + } +} + +const std::string LLDiskCache::assetTypeToString(LLAssetType::EType at) +{ + /** + * Make use of the handy C++17 feature that allows + * for inline initialization of an std::map<> + */ + typedef std::map asset_type_to_name_t; + asset_type_to_name_t asset_type_to_name = + { + { LLAssetType::AT_TEXTURE, "TEXTURE" }, + { LLAssetType::AT_SOUND, "SOUND" }, + { LLAssetType::AT_CALLINGCARD, "CALLINGCARD" }, + { LLAssetType::AT_LANDMARK, "LANDMARK" }, + { LLAssetType::AT_SCRIPT, "SCRIPT" }, + { LLAssetType::AT_CLOTHING, "CLOTHING" }, + { LLAssetType::AT_OBJECT, "OBJECT" }, + { LLAssetType::AT_NOTECARD, "NOTECARD" }, + { LLAssetType::AT_CATEGORY, "CATEGORY" }, + { LLAssetType::AT_LSL_TEXT, "LSL_TEXT" }, + { LLAssetType::AT_LSL_BYTECODE, "LSL_BYTECODE" }, + { LLAssetType::AT_TEXTURE_TGA, "TEXTURE_TGA" }, + { LLAssetType::AT_BODYPART, "BODYPART" }, + { LLAssetType::AT_SOUND_WAV, "SOUND_WAV" }, + { LLAssetType::AT_IMAGE_TGA, "IMAGE_TGA" }, + { LLAssetType::AT_IMAGE_JPEG, "IMAGE_JPEG" }, + { LLAssetType::AT_ANIMATION, "ANIMATION" }, + { LLAssetType::AT_GESTURE, "GESTURE" }, + { LLAssetType::AT_SIMSTATE, "SIMSTATE" }, + { LLAssetType::AT_LINK, "LINK" }, + { LLAssetType::AT_LINK_FOLDER, "LINK_FOLDER" }, + { LLAssetType::AT_MARKETPLACE_FOLDER, "MARKETPLACE_FOLDER" }, + { LLAssetType::AT_WIDGET, "WIDGET" }, + { LLAssetType::AT_PERSON, "PERSON" }, + { LLAssetType::AT_MESH, "MESH" }, + { LLAssetType::AT_SETTINGS, "SETTINGS" }, + { LLAssetType::AT_UNKNOWN, "UNKNOWN" } + }; + + asset_type_to_name_t::iterator iter = asset_type_to_name.find(at); + if (iter != asset_type_to_name.end()) + { + return iter->second; + } + + return std::string("UNKNOWN"); +} + +const std::string LLDiskCache::metaDataToFilepath(const std::string id, + LLAssetType::EType at, + const std::string extra_info) +{ + std::ostringstream file_path; + + file_path << mCacheDir; + file_path << gDirUtilp->getDirDelimiter(); + file_path << mCacheFilenamePrefix; + file_path << "_"; + file_path << id; + file_path << "_"; + file_path << (extra_info.empty() ? "0" : extra_info); + //file_path << "_"; + //file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames + // for details of why it was removed. Note that if you put it + // back or change the format of the filename, the cache files + // files will be invalidated (and perhaps, more importantly, + // never deleted unless you delete them manually). + file_path << ".asset"; + + return file_path.str(); +} + +void LLDiskCache::updateFileAccessTime(const std::string file_path) +{ + /** + * Threshold in time_t units that is used to decide if the last access time + * time of the file is updated or not. Added as a precaution for the concern + * outlined in SL-14582 about frequent writes on older SSDs reducing their + * lifespan. I think this is the right place for the threshold value - rather + * than it being a pref - do comment on that Jira if you disagree... + * + * Let's start with 1 hour in time_t units and see how that unfolds + */ + const std::time_t time_threshold = 1 * 60 * 60; + + // current time + const std::time_t cur_time = std::time(nullptr); + +#if LL_WINDOWS + // file last write time + const std::time_t last_write_time = boost::filesystem::last_write_time(utf8str_to_utf16str(file_path)); + + // delta between cur time and last time the file was written + const std::time_t delta_time = cur_time - last_write_time; + + // we only write the new value if the time in time_threshold has elapsed + // before the last one + if (delta_time > time_threshold) + { + boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), cur_time); + } +#else + // file last write time + const std::time_t last_write_time = boost::filesystem::last_write_time(file_path); + + // delta between cur time and last time the file was written + const std::time_t delta_time = cur_time - last_write_time; + + // we only write the new value if the time in time_threshold has elapsed + // before the last one + if (delta_time > time_threshold) + { + boost::filesystem::last_write_time(file_path, cur_time); + } +#endif +} + +const std::string LLDiskCache::getCacheInfo() +{ + std::ostringstream cache_info; + + F32 max_in_mb = (F32)mMaxSizeBytes / (1024.0 * 1024.0); + F32 percent_used = ((F32)dirFileSize(mCacheDir) / (F32)mMaxSizeBytes) * 100.0; + + cache_info << std::fixed; + cache_info << std::setprecision(1); + cache_info << "Max size " << max_in_mb << " MB "; + cache_info << "(" << percent_used << "% used)"; + + return cache_info.str(); +} + +void LLDiskCache::clearCache() +{ + /** + * See notes on performance in dirFileSize(..) - there may be + * a quicker way to do this by operating on the parent dir vs + * the component files but it's called infrequently so it's + * likely just fine + */ +#if LL_WINDOWS + std::wstring cache_path(utf8str_to_utf16str(mCacheDir)); +#else + std::string cache_path(mCacheDir); +#endif + if (boost::filesystem::is_directory(cache_path)) + { + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + { + if (boost::filesystem::is_regular_file(entry)) + { + if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) + { + boost::filesystem::remove(entry); + } + } + } + } +} + +uintmax_t LLDiskCache::dirFileSize(const std::string dir) +{ + uintmax_t total_file_size = 0; + + /** + * There may be a better way that works directly on the folder (similar to + * right clicking on a folder in the OS and asking for size vs right clicking + * on all files and adding up manually) but this is very fast - less than 100ms + * for 10,000 files in my testing so, so long as it's not called frequently, + * it should be okay. Note that's it's only currently used for logging/debugging + * so if performance is ever an issue, optimizing this or removing it altogether, + * is an easy win. + */ +#if LL_WINDOWS + std::wstring dir_path(utf8str_to_utf16str(dir)); +#else + std::string dir_path(dir); +#endif + if (boost::filesystem::is_directory(dir_path)) + { + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path), {})) + { + if (boost::filesystem::is_regular_file(entry)) + { + if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) + { + total_file_size += boost::filesystem::file_size(entry); + } + } + } + } + + return total_file_size; +} -- cgit v1.3 From d7518c7b4f5eca731d0ea143bab34b279bd4ee13 Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 9 Mar 2021 18:33:35 -0800 Subject: Ansariel kindly offered their patch to help mitigate this round of file system issues - taken from https://vcs.firestormviewer.org/phoenix-firestorm/changeset/104a8600946be01e2de44d10ad069ba854272d1f --- indra/llfilesystem/lldiskcache.cpp | 4 ++-- indra/llfilesystem/llfilesystem.cpp | 27 +++++++++++++++++++++++++++ indra/newview/llmeshrepository.cpp | 34 +++++++++++++++++++++++++++++----- 3 files changed, 58 insertions(+), 7 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c9f7684b5a..61eb654693 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -192,8 +192,8 @@ const std::string LLDiskCache::metaDataToFilepath(const std::string id, file_path << id; file_path << "_"; file_path << (extra_info.empty() ? "0" : extra_info); - //file_path << "_"; - //file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames + file_path << "_"; + file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames // for details of why it was removed. Note that if you put it // back or change the format of the filename, the cache files // files will be invalidated (and perhaps, more importantly, diff --git a/indra/llfilesystem/llfilesystem.cpp b/indra/llfilesystem/llfilesystem.cpp index 053b52014e..da44e8d98c 100644 --- a/indra/llfilesystem/llfilesystem.cpp +++ b/indra/llfilesystem/llfilesystem.cpp @@ -200,9 +200,36 @@ BOOL LLFileSystem::write(const U8* buffer, S32 bytes) { ofs.write((const char*)buffer, bytes); + mPosition = ofs.tellp(); // Fix asset caching + + success = TRUE; + } + } + // Fix asset caching + else if (mMode == READ_WRITE) + { + // Don't truncate if file already exists + llofstream ofs(filename, std::ios::in | std::ios::binary); + if (ofs) + { + ofs.seekp(mPosition, std::ios::beg); + ofs.write((const char*)buffer, bytes); + mPosition += bytes; success = TRUE; } + else + { + // File doesn't exist - open in write mode + ofs.open(filename, std::ios::binary); + if (ofs.is_open()) + { + ofs.write((const char*)buffer, bytes); + mPosition += bytes; + success = TRUE; + } + } } + // else { llofstream ofs(filename, std::ios::binary); diff --git a/indra/newview/llmeshrepository.cpp b/indra/newview/llmeshrepository.cpp index 8e5bdc0225..c2404a7e67 100644 --- a/indra/newview/llmeshrepository.cpp +++ b/indra/newview/llmeshrepository.cpp @@ -3248,13 +3248,29 @@ void LLMeshHeaderHandler::processData(LLCore::BufferArray * /* body */, S32 /* b // only allocate as much space in the cache as is needed for the local cache data_size = llmin(data_size, bytes); - LLFileSystem file(mesh_id, LLAssetType::AT_MESH, LLFileSystem::WRITE); + // Fix asset caching + //LLFileSystem file(mesh_id, LLAssetType::AT_MESH, LLFileSystem::WRITE); + LLFileSystem file(mesh_id, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE); if (file.getMaxSize() >= bytes) { LLMeshRepository::sCacheBytesWritten += data_size; ++LLMeshRepository::sCacheWrites; file.write(data, data_size); + + // Fix asset caching + S32 remaining = bytes - file.tell(); + if (remaining > 0) + { + U8* block = new(std::nothrow) U8[remaining]; + if (block) + { + memset(block, 0, remaining); + file.write(block, remaining); + delete[] block; + } + } + // } } else @@ -3307,7 +3323,9 @@ void LLMeshLODHandler::processData(LLCore::BufferArray * /* body */, S32 /* body if (result == MESH_OK) { // good fetch from sim, write to cache - LLFileSystem file(mMeshParams.getSculptID(), LLAssetType::AT_MESH, LLFileSystem::WRITE); + // Fix asset caching + //LLFileSystem file(mMeshParams.getSculptID(), LLAssetType::AT_MESH, LLFileSystem::WRITE); + LLFileSystem file(mMeshParams.getSculptID(), LLAssetType::AT_MESH, LLFileSystem::READ_WRITE); S32 offset = mOffset; S32 size = mRequestedBytes; @@ -3371,7 +3389,9 @@ void LLMeshSkinInfoHandler::processData(LLCore::BufferArray * /* body */, S32 /* && gMeshRepo.mThread->skinInfoReceived(mMeshID, data, data_size)) { // good fetch from sim, write to cache - LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE); + // Fix asset caching + //LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE); + LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE); S32 offset = mOffset; S32 size = mRequestedBytes; @@ -3419,7 +3439,9 @@ void LLMeshDecompositionHandler::processData(LLCore::BufferArray * /* body */, S && gMeshRepo.mThread->decompositionReceived(mMeshID, data, data_size)) { // good fetch from sim, write to cache - LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE); + // Fix asset caching + //LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE); + LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE); S32 offset = mOffset; S32 size = mRequestedBytes; @@ -3466,7 +3488,9 @@ void LLMeshPhysicsShapeHandler::processData(LLCore::BufferArray * /* body */, S3 && gMeshRepo.mThread->physicsShapeReceived(mMeshID, data, data_size) == MESH_OK) { // good fetch from sim, write to cache for caching - LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE); + // Fix asset caching + //LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE); + LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE); S32 offset = mOffset; S32 size = mRequestedBytes; -- cgit v1.3 From ad2edc1d8336c745fc897197e2e85debf3654c17 Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 9 Mar 2021 18:35:30 -0800 Subject: Remove debugging tags --- indra/llfilesystem/lldiskcache.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 61eb654693..c9f7684b5a 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -192,8 +192,8 @@ const std::string LLDiskCache::metaDataToFilepath(const std::string id, file_path << id; file_path << "_"; file_path << (extra_info.empty() ? "0" : extra_info); - file_path << "_"; - file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames + //file_path << "_"; + //file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames // for details of why it was removed. Note that if you put it // back or change the format of the filename, the cache files // files will be invalidated (and perhaps, more importantly, -- cgit v1.3 From 8633ba9c8bd8e24f9b6d8a74ed7bc4d50072a728 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Tue, 11 May 2021 20:27:07 +0200 Subject: BUG-230697: Do not crash viewer during cache cleanup --- indra/llfilesystem/lldiskcache.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c9f7684b5a..75255ef230 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -102,7 +102,12 @@ void LLDiskCache::purge() if (file_size_total > mMaxSizeBytes) { action = "DELETE:"; - boost::filesystem::remove(entry.second.second); + boost::system::error_code ec; + boost::filesystem::remove(entry.second.second, ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to delete cache file " << entry.second.second << ": " << ec.message() << LL_ENDL; + } } else { @@ -284,7 +289,12 @@ void LLDiskCache::clearCache() { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - boost::filesystem::remove(entry); + boost::system::error_code ec; + boost::filesystem::remove(entry, ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to delete cache file " << entry << ": " << ec.message() << LL_ENDL; + } } } } -- cgit v1.3 From 3898609ae24e7787d6f6ae71c2424b43102326bf Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 11 May 2021 12:58:05 -0700 Subject: Fix for SL-15226 Simple cache viewer: Integer overflow in cache size - via FS:Ansariel --- indra/llfilesystem/lldiskcache.cpp | 2 +- indra/llfilesystem/lldiskcache.h | 2 +- indra/newview/llappviewer.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c9f7684b5a..241c46dc73 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -40,7 +40,7 @@ #include "lldiskcache.h" LLDiskCache::LLDiskCache(const std::string cache_dir, - const int max_size_bytes, + const uintmax_t max_size_bytes, const bool enable_cache_debug_info) : mCacheDir(cache_dir), mMaxSizeBytes(max_size_bytes), diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h index 997884da31..c19714434a 100644 --- a/indra/llfilesystem/lldiskcache.h +++ b/indra/llfilesystem/lldiskcache.h @@ -86,7 +86,7 @@ class LLDiskCache : * The maximum size of the cache in bytes - Based on the * setting at 'CacheSize' and 'DiskCachePercentOfTotal' */ - const int max_size_bytes, + const uintmax_t max_size_bytes, /** * A flag that enables extra cache debugging so that * if there are bugs, we can ask uses to enable this diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index a3d20a8e2f..c519e55fc3 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -4141,7 +4141,7 @@ bool LLAppViewer::initCache() const unsigned int cache_total_size_mb = gSavedSettings.getU32("CacheSize"); const double disk_cache_percent = gSavedSettings.getF32("DiskCachePercentOfTotal"); const unsigned int disk_cache_mb = cache_total_size_mb * disk_cache_percent / 100; - const unsigned int disk_cache_bytes = disk_cache_mb * 1024 * 1024; + const uintmax_t disk_cache_bytes = disk_cache_mb * 1024 * 1024; const bool enable_cache_debug_info = gSavedSettings.getBOOL("EnableDiskCacheDebugInfo"); bool texture_cache_mismatch = false; -- cgit v1.3 From 0e253cb9098cb58b2f3528860a5fd9f00ec5af96 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Wed, 12 May 2021 10:45:23 +0200 Subject: BUG-230673: Trim asset disk cache regularly --- indra/llfilesystem/lldiskcache.cpp | 22 ++++++++++++++++++++++ indra/llfilesystem/lldiskcache.h | 11 +++++++++++ indra/newview/app_settings/settings.xml | 2 +- indra/newview/llappviewer.cpp | 6 ++++++ indra/newview/llappviewer.h | 3 +++ 5 files changed, 43 insertions(+), 1 deletion(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 02678864b9..68423cc4f8 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -335,3 +335,25 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir) return total_file_size; } + +LLPurgeDiskCacheThread::LLPurgeDiskCacheThread() : + LLThread("PurgeDiskCacheThread", nullptr) +{ +} + +void LLPurgeDiskCacheThread::run() +{ + constexpr F64 CHECK_INTERVAL = 60; + mTimer.setTimerExpirySec(CHECK_INTERVAL); + mTimer.start(); + + do + { + if (mTimer.checkExpirationAndReset(CHECK_INTERVAL)) + { + LLDiskCache::instance().purge(); + } + + ms_sleep(100); + } while (!isQuitting()); +} \ No newline at end of file diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h index c19714434a..867a80f332 100644 --- a/indra/llfilesystem/lldiskcache.h +++ b/indra/llfilesystem/lldiskcache.h @@ -180,4 +180,15 @@ class LLDiskCache : bool mEnableCacheDebugInfo; }; +class LLPurgeDiskCacheThread : public LLThread +{ +public: + LLPurgeDiskCacheThread(); + +protected: + void run() override; + +private: + LLTimer mTimer; +}; #endif // _LLDISKCACHE diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml index cbd33e9244..0cae9cd9cc 100644 --- a/indra/newview/app_settings/settings.xml +++ b/indra/newview/app_settings/settings.xml @@ -1362,7 +1362,7 @@ Value 23 - EnableCacheDebugInfo + EnableDiskCacheDebugInfo Comment When set, display additional cache debugging information diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index c519e55fc3..fd094b12d7 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -655,6 +655,7 @@ LLAppViewer* LLAppViewer::sInstance = NULL; LLTextureCache* LLAppViewer::sTextureCache = NULL; LLImageDecodeThread* LLAppViewer::sImageDecodeThread = NULL; LLTextureFetch* LLAppViewer::sTextureFetch = NULL; +LLPurgeDiskCacheThread* LLAppViewer::sPurgeDiskCacheThread = NULL; std::string getRuntime() { @@ -2032,6 +2033,7 @@ bool LLAppViewer::cleanup() sTextureFetch->shutdown(); sTextureCache->shutdown(); sImageDecodeThread->shutdown(); + sPurgeDiskCacheThread->shutdown(); sTextureFetch->shutDownTextureCacheThread() ; sTextureFetch->shutDownImageDecodeThread() ; @@ -2054,6 +2056,8 @@ bool LLAppViewer::cleanup() sImageDecodeThread = NULL; delete mFastTimerLogThread; mFastTimerLogThread = NULL; + delete sPurgeDiskCacheThread; + sPurgeDiskCacheThread = NULL; if (LLFastTimerView::sAnalyzePerformance) { @@ -2174,6 +2178,7 @@ bool LLAppViewer::initThreads() sImageDecodeThread, enable_threads && true, app_metrics_qa_mode); + LLAppViewer::sPurgeDiskCacheThread = new LLPurgeDiskCacheThread(); if (LLTrace::BlockTimer::sLog || LLTrace::BlockTimer::sMetricLog) { @@ -4210,6 +4215,7 @@ bool LLAppViewer::initCache() LLDiskCache::getInstance()->purge(); } } + LLAppViewer::getPurgeDiskCacheThread()->start(); LLSplashScreen::update(LLTrans::getString("StartupInitializingTextureCache")); diff --git a/indra/newview/llappviewer.h b/indra/newview/llappviewer.h index 5e24398caa..00d6943047 100644 --- a/indra/newview/llappviewer.h +++ b/indra/newview/llappviewer.h @@ -58,6 +58,7 @@ class LLImageDecodeThread; class LLTextureFetch; class LLWatchdogTimeout; class LLViewerJoystick; +class LLPurgeDiskCacheThread; extern LLTrace::BlockTimerStatHandle FTM_FRAME; @@ -117,6 +118,7 @@ public: static LLTextureCache* getTextureCache() { return sTextureCache; } static LLImageDecodeThread* getImageDecodeThread() { return sImageDecodeThread; } static LLTextureFetch* getTextureFetch() { return sTextureFetch; } + static LLPurgeDiskCacheThread* getPurgeDiskCacheThread() { return sPurgeDiskCacheThread; } static U32 getTextureCacheVersion() ; static U32 getObjectCacheVersion() ; @@ -284,6 +286,7 @@ private: static LLTextureCache* sTextureCache; static LLImageDecodeThread* sImageDecodeThread; static LLTextureFetch* sTextureFetch; + static LLPurgeDiskCacheThread* sPurgeDiskCacheThread; S32 mNumSessions; -- cgit v1.3 From 89cf988aaf0de402641cd945e7e9ad5292bc78d6 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Mon, 17 May 2021 09:49:32 +0200 Subject: BUG-230673: Add warning that LLDiskCache::purge() is also called from outside the main thread --- indra/llfilesystem/lldiskcache.cpp | 2 ++ indra/llfilesystem/lldiskcache.h | 3 +++ 2 files changed, 5 insertions(+) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 68423cc4f8..17906ce369 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -51,6 +51,8 @@ LLDiskCache::LLDiskCache(const std::string cache_dir, LLFile::mkdir(cache_dir); } +// WARNING: purge() is called by LLPurgeDiskCacheThread. As such it must +// NOT touch any LLDiskCache data without introducing and locking a mutex! void LLDiskCache::purge() { if (mEnableCacheDebugInfo) diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h index 867a80f332..7c5b798f7e 100644 --- a/indra/llfilesystem/lldiskcache.h +++ b/indra/llfilesystem/lldiskcache.h @@ -118,6 +118,9 @@ class LLDiskCache : /** * Purge the oldest items in the cache so that the combined size of all files * is no bigger than mMaxSizeBytes. + * + * WARNING: purge() is called by LLPurgeDiskCacheThread. As such it must + * NOT touch any LLDiskCache data without introducing and locking a mutex! */ void purge(); -- cgit v1.3 From 87faf258911f5d23416500ff632050ce05b30e3e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 May 2021 10:24:27 -0400 Subject: SL-15200: Explain why purge() is called on another thread. Also add Ansariel's explanation for why interaction through the filesystem itself should be safe. --- indra/llfilesystem/lldiskcache.cpp | 28 +++++++++++++++++++++++++++- indra/llfilesystem/lldiskcache.h | 4 ++++ 2 files changed, 31 insertions(+), 1 deletion(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 17906ce369..c0f10ac620 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -53,6 +53,32 @@ LLDiskCache::LLDiskCache(const std::string cache_dir, // WARNING: purge() is called by LLPurgeDiskCacheThread. As such it must // NOT touch any LLDiskCache data without introducing and locking a mutex! + +// Interaction through the filesystem itself should be safe. Let’s say thread +// A is accessing the cache file for reading/writing and thread B is trimming +// the cache. Let’s also assume using llifstream to open a file and +// boost::filesystem::remove are not atomic (which will be pretty much the +// case). + +// Now, A is trying to open the file using llifstream ctor. It does some +// checks if the file exists and whatever else it might be doing, but has not +// issued the call to the OS to actually open the file yet. Now B tries to +// delete the file: If the file has been already marked as in use by the OS, +// deleting the file will fail and B will continue with the next file. A can +// safely continue opening the file. If the file has not yet been marked as in +// use, B will delete the file. Now A actually wants to open it, operation +// will fail, subsequent check via llifstream.is_open will fail, asset will +// have to be re-requested. (Assuming here the viewer will actually handle +// this situation properly, that can also happen if there is a file containing +// garbage.) + +// Other situation: B is trimming the cache and A wants to read a file that is +// about to get deleted. boost::filesystem::remove does whatever it is doing +// before actually deleting the file. If A opens the file before the file is +// actually gone, the OS call from B to delete the file will fail since the OS +// will prevent this. B continues with the next file. If the file is already +// gone before A finally gets to open it, this operation will fail and the +// asset will have to be re-requested. void LLDiskCache::purge() { if (mEnableCacheDebugInfo) @@ -358,4 +384,4 @@ void LLPurgeDiskCacheThread::run() ms_sleep(100); } while (!isQuitting()); -} \ No newline at end of file +} diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h index 7c5b798f7e..268fe92bcc 100644 --- a/indra/llfilesystem/lldiskcache.h +++ b/indra/llfilesystem/lldiskcache.h @@ -121,6 +121,10 @@ class LLDiskCache : * * WARNING: purge() is called by LLPurgeDiskCacheThread. As such it must * NOT touch any LLDiskCache data without introducing and locking a mutex! + * + * Purging the disk cache involves nontrivial work on the viewer's + * filesystem. If called on the main thread, this causes a noticeable + * freeze. */ void purge(); -- cgit v1.3 From b3708ac238d51eaf808cb77a4493e518c1593e33 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 May 2021 15:10:06 -0400 Subject: SL-15200: Use new LLApp::sleep() in LLPurgeDiskCacheThread::run(). --- indra/llfilesystem/lldiskcache.cpp | 16 +++++----------- indra/llfilesystem/lldiskcache.h | 3 --- 2 files changed, 5 insertions(+), 14 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c0f10ac620..6b0bbaeb48 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -31,6 +31,7 @@ */ #include "linden_common.h" +#include "llapp.h" #include "llassettype.h" #include "lldir.h" #include @@ -371,17 +372,10 @@ LLPurgeDiskCacheThread::LLPurgeDiskCacheThread() : void LLPurgeDiskCacheThread::run() { - constexpr F64 CHECK_INTERVAL = 60; - mTimer.setTimerExpirySec(CHECK_INTERVAL); - mTimer.start(); + constexpr long CHECK_INTERVAL = 60; - do + while (LLApp::instance()->sleep(std::chrono::seconds(CHECK_INTERVAL))) { - if (mTimer.checkExpirationAndReset(CHECK_INTERVAL)) - { - LLDiskCache::instance().purge(); - } - - ms_sleep(100); - } while (!isQuitting()); + LLDiskCache::instance().purge(); + } } diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h index 268fe92bcc..1cbd2c58aa 100644 --- a/indra/llfilesystem/lldiskcache.h +++ b/indra/llfilesystem/lldiskcache.h @@ -194,8 +194,5 @@ public: protected: void run() override; - -private: - LLTimer mTimer; }; #endif // _LLDISKCACHE -- cgit v1.3 From ac8640d338997020ca0650001ff004e1103ac5cb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 18 May 2021 09:51:45 -0400 Subject: SL-15200: LLPurgeDiskCacheThread's CHECK_INTERVAL is secs. --- indra/llfilesystem/lldiskcache.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 6b0bbaeb48..905351886f 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -372,9 +372,9 @@ LLPurgeDiskCacheThread::LLPurgeDiskCacheThread() : void LLPurgeDiskCacheThread::run() { - constexpr long CHECK_INTERVAL = 60; + constexpr std::chrono::seconds CHECK_INTERVAL{60}; - while (LLApp::instance()->sleep(std::chrono::seconds(CHECK_INTERVAL))) + while (LLApp::instance()->sleep(CHECK_INTERVAL)) { LLDiskCache::instance().purge(); } -- cgit v1.3 From 4c558e85bd90d77696e9201b8996272176d7adc5 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Thu, 10 Jun 2021 01:09:31 +0200 Subject: Fix more crashes in disk cache due to boost error handling --- indra/llfilesystem/lldiskcache.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 905351886f..f4d20d0e61 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -253,9 +253,15 @@ void LLDiskCache::updateFileAccessTime(const std::string file_path) // current time const std::time_t cur_time = std::time(nullptr); + boost::system::error_code ec; #if LL_WINDOWS // file last write time - const std::time_t last_write_time = boost::filesystem::last_write_time(utf8str_to_utf16str(file_path)); + const std::time_t last_write_time = boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to read last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL; + return; + } // delta between cur time and last time the file was written const std::time_t delta_time = cur_time - last_write_time; @@ -264,11 +270,16 @@ void LLDiskCache::updateFileAccessTime(const std::string file_path) // before the last one if (delta_time > time_threshold) { - boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), cur_time); + boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), cur_time, ec); } #else // file last write time - const std::time_t last_write_time = boost::filesystem::last_write_time(file_path); + const std::time_t last_write_time = boost::filesystem::last_write_time(file_path, ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to read last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL; + return; + } // delta between cur time and last time the file was written const std::time_t delta_time = cur_time - last_write_time; @@ -277,9 +288,14 @@ void LLDiskCache::updateFileAccessTime(const std::string file_path) // before the last one if (delta_time > time_threshold) { - boost::filesystem::last_write_time(file_path, cur_time); + boost::filesystem::last_write_time(file_path, cur_time, ec); } #endif + + if (ec.failed()) + { + LL_WARNS() << "Failed to update last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL; + } } const std::string LLDiskCache::getCacheInfo() -- cgit v1.3 From 564a7acb321c54ad5f9fc6f5242efbe4c7638dbc Mon Sep 17 00:00:00 2001 From: Ansariel Date: Fri, 11 Jun 2021 22:50:39 +0200 Subject: Change all remaining boost::filesystem methods to their non-throwing overloads --- indra/llfilesystem/lldiskcache.cpp | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index f4d20d0e61..dd8916dccb 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -87,6 +87,7 @@ void LLDiskCache::purge() LL_INFOS() << "Total dir size before purge is " << dirFileSize(mCacheDir) << LL_ENDL; } + boost::system::error_code ec; auto start_time = std::chrono::high_resolution_clock::now(); typedef std::pair> file_info_t; @@ -97,17 +98,25 @@ void LLDiskCache::purge() #else std::string cache_path(mCacheDir); #endif - if (boost::filesystem::is_directory(cache_path)) + if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) { - if (boost::filesystem::is_regular_file(entry)) + if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - uintmax_t file_size = boost::filesystem::file_size(entry); + uintmax_t file_size = boost::filesystem::file_size(entry, ec); + if (ec.failed()) + { + continue; + } const std::string file_path = entry.path().string(); - const std::time_t file_time = boost::filesystem::last_write_time(entry); + const std::time_t file_time = boost::filesystem::last_write_time(entry, ec); + if (ec.failed()) + { + continue; + } file_info.push_back(file_info_t(file_time, { file_size, file_path })); } @@ -131,7 +140,6 @@ void LLDiskCache::purge() if (file_size_total > mMaxSizeBytes) { action = "DELETE:"; - boost::system::error_code ec; boost::filesystem::remove(entry.second.second, ec); if (ec.failed()) { @@ -321,20 +329,20 @@ void LLDiskCache::clearCache() * the component files but it's called infrequently so it's * likely just fine */ + boost::system::error_code ec; #if LL_WINDOWS std::wstring cache_path(utf8str_to_utf16str(mCacheDir)); #else std::string cache_path(mCacheDir); #endif - if (boost::filesystem::is_directory(cache_path)) + if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) { - if (boost::filesystem::is_regular_file(entry)) + if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - boost::system::error_code ec; boost::filesystem::remove(entry, ec); if (ec.failed()) { @@ -359,20 +367,25 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir) * so if performance is ever an issue, optimizing this or removing it altogether, * is an easy win. */ + boost::system::error_code ec; #if LL_WINDOWS std::wstring dir_path(utf8str_to_utf16str(dir)); #else std::string dir_path(dir); #endif - if (boost::filesystem::is_directory(dir_path)) + if (boost::filesystem::is_directory(dir_path, ec) && !ec.failed()) { for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path), {})) { - if (boost::filesystem::is_regular_file(entry)) + if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - total_file_size += boost::filesystem::file_size(entry); + uintmax_t file_size = boost::filesystem::file_size(entry, ec); + if (!ec.failed()) + { + total_file_size += file_size; + } } } } -- cgit v1.3 From fedae88be7b7174956f549194b13883ff2cb4161 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Fri, 11 Jun 2021 22:59:55 +0200 Subject: boost::filesystem::directory_iterator uses throw-behavior by default as well --- indra/llfilesystem/lldiskcache.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index dd8916dccb..ee43a599f7 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -100,7 +100,7 @@ void LLDiskCache::purge() #endif if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { - for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path, ec), {})) { if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { @@ -337,7 +337,7 @@ void LLDiskCache::clearCache() #endif if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { - for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path, ec), {})) { if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { @@ -375,7 +375,7 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir) #endif if (boost::filesystem::is_directory(dir_path, ec) && !ec.failed()) { - for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path), {})) + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path, ec), {})) { if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { -- cgit v1.3