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.h | 183 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 indra/llfilesystem/lldiskcache.h (limited to 'indra/llfilesystem/lldiskcache.h') diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h new file mode 100644 index 0000000000..997884da31 --- /dev/null +++ b/indra/llfilesystem/lldiskcache.h @@ -0,0 +1,183 @@ +/** + * @file lldiskcache.h + * @brief The disk cache implementation declarations. + * + * @Description: + * This code implements a disk cache using the following ideas: + * 1/ The metadata for a file can be encapsulated in the filename. + The filenames will be composed of the following fields: + Prefix: Used to identify the file as a part of the cache. + An additional reason for using a prefix is that it + might be possible, either accidentally or maliciously + to end up with the cache dir set to a non-cache + location such as your OS system dir or a work folder. + Purging files from that would obviously be a disaster + so this is an extra step to help avoid that scenario. + ID: Typically the asset ID (UUID) of the asset being + saved but can be anything valid for a filename + Extra Info: A field for use in the future that can be used + to store extra identifiers - e.g. the discard + level of a JPEG2000 file + Asset Type: A text string created from the LLAssetType enum + that identifies the type of asset being stored. + .asset A file extension of .asset is used to help + identify this as a Viewer asset file + * 2/ The time of last access for a file can be updated instantly + * for file reads and automatically as part of the file writes. + * 3/ The purge algorithm collects a list of all files in the + * directory, sorts them by date of last access (write) and then + * deletes any files based on age until the total size of all + * the files is less than the maximum size specified. + * 4/ An LLSingleton idiom is used since there will only ever be + * a single cache and we want to access it from numerous places. + * 5/ Performance on my modest system seems very acceptable. For + * example, in testing, I was able to purge a directory of + * 10,000 files, deleting about half of them in ~ 1700ms. For + * the same sized directory of files, writing the last updated + * time to each took less than 600ms indicating that this + * important part of the mechanism has almost no overhead. + * + * $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$ + */ + +#ifndef _LLDISKCACHE +#define _LLDISKCACHE + +#include "llsingleton.h" + +class LLDiskCache : + public LLParamSingleton +{ + public: + /** + * Since this is using the LLSingleton pattern but we + * want to allow the constructor to be called first + * with various parameters, we also invoke the + * LLParamSingleton idiom and use it to initialize + * the class via a call in LLAppViewer. + */ + LLSINGLETON(LLDiskCache, + /** + * The full name of the cache folder - typically a + * a child of the main Viewer cache directory. Defined + * by the setting at 'DiskCacheDirName' + */ + const std::string cache_dir, + /** + * The maximum size of the cache in bytes - Based on the + * setting at 'CacheSize' and 'DiskCachePercentOfTotal' + */ + const int max_size_bytes, + /** + * A flag that enables extra cache debugging so that + * if there are bugs, we can ask uses to enable this + * setting and send us their logs + */ + const bool enable_cache_debug_info); + + virtual ~LLDiskCache() = default; + + public: + /** + * Construct a filename and path to it based on the file meta data + * (id, asset type, additional 'extra' info like discard level perhaps) + * Worth pointing out that this function used to be in LLFileSystem but + * so many things had to be pushed back there to accomodate it, that I + * decided to move it here. Still not sure that's completely right. + */ + const std::string metaDataToFilepath(const std::string id, + LLAssetType::EType at, + const std::string extra_info); + + /** + * Update the "last write time" of a file to "now". This must be called whenever a + * file in the cache is read (not written) so that the last time the file was + * accessed is up to date (This is used in the mechanism for purging the cache) + */ + void updateFileAccessTime(const std::string file_path); + + /** + * Purge the oldest items in the cache so that the combined size of all files + * is no bigger than mMaxSizeBytes. + */ + void purge(); + + /** + * Clear the cache by removing all the files in the specified cache + * directory individually. Only the files that contain a prefix defined + * by mCacheFilenamePrefix will be removed. + */ + void clearCache(); + + /** + * Return some information about the cache for use in About Box etc. + */ + const std::string getCacheInfo(); + + private: + /** + * Utility function to gather the total size the files in a given + * directory. Primarily used here to determine the directory size + * before and after the cache purge + */ + uintmax_t dirFileSize(const std::string dir); + + /** + * Utility function to convert an LLAssetType enum into a + * string that we use as part of the cache file filename + */ + const std::string assetTypeToString(LLAssetType::EType at); + + private: + /** + * The maximum size of the cache in bytes. After purge is called, the + * total size of the cache files in the cache directory will be + * less than this value + */ + uintmax_t mMaxSizeBytes; + + /** + * The folder that holds the cached files. The consumer of this + * class must avoid letting the user set this location as a malicious + * setting could potentially point it at a non-cache directory (for example, + * the Windows System dir) with disastrous results. + */ + std::string mCacheDir; + + /** + * The prefix inserted at the start of a cache file filename to + * help identify it as a cache file. It's probably not required + * (just the presence in the cache folder is enough) but I am + * paranoid about the cache folder being set to something bad + * like the users' OS system dir by mistake or maliciously and + * this will help to offset any damage if that happens. + */ + std::string mCacheFilenamePrefix; + + /** + * When enabled, displays additional debugging information in + * various parts of the code + */ + bool mEnableCacheDebugInfo; +}; + +#endif // _LLDISKCACHE -- 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.h') 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.h') 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.h') 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.h') 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.h') 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