Mitigate asset fetch thread stalls from LLDiskCache mutex contention and trivial cleanup

Move LLDiskCache::updateFileAccessTime to LLFilesystem as it's the only user of that function.

Change mCacheDir and LLDiskCache::metaDataToFilepath to statics.
meow-7.2.2
Rye Mutt 2024-08-13 18:35:46 -04:00
parent 6dbf1cafb2
commit 0ee528c5fd
4 changed files with 108 additions and 132 deletions

View File

@ -39,15 +39,25 @@
#include "lldiskcache.h"
/**
* 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.
*/
static const std::string CACHE_FILENAME_PREFIX("sl_cache");
std::string LLDiskCache::sCacheDir;
LLDiskCache::LLDiskCache(const std::string cache_dir,
const uintmax_t max_size_bytes,
const bool enable_cache_debug_info) :
mCacheDir(cache_dir),
mMaxSizeBytes(max_size_bytes),
mEnableCacheDebugInfo(enable_cache_debug_info)
{
mCacheFilenamePrefix = "sl_cache";
sCacheDir = cache_dir;
LLFile::mkdir(cache_dir);
}
@ -83,7 +93,7 @@ void LLDiskCache::purge()
{
if (mEnableCacheDebugInfo)
{
LL_INFOS() << "Total dir size before purge is " << dirFileSize(mCacheDir) << LL_ENDL;
LL_INFOS() << "Total dir size before purge is " << dirFileSize(sCacheDir) << LL_ENDL;
}
boost::system::error_code ec;
@ -93,9 +103,9 @@ void LLDiskCache::purge()
std::vector<file_info_t> file_info;
#if LL_WINDOWS
std::wstring cache_path(utf8str_to_utf16str(mCacheDir));
std::wstring cache_path(utf8str_to_utf16str(sCacheDir));
#else
std::string cache_path(mCacheDir);
std::string cache_path(sCacheDir);
#endif
if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed())
{
@ -104,7 +114,7 @@ void LLDiskCache::purge()
{
if (boost::filesystem::is_regular_file(*iter, ec) && !ec.failed())
{
if ((*iter).path().string().find(mCacheFilenamePrefix) != std::string::npos)
if ((*iter).path().string().find(CACHE_FILENAME_PREFIX) != std::string::npos)
{
uintmax_t file_size = boost::filesystem::file_size(*iter, ec);
if (ec.failed())
@ -181,7 +191,7 @@ void LLDiskCache::purge()
LL_INFOS() << line.str() << LL_ENDL;
}
LL_INFOS() << "Total dir size after purge is " << dirFileSize(mCacheDir) << LL_ENDL;
LL_INFOS() << "Total dir size after purge is " << dirFileSize(sCacheDir) << LL_ENDL;
LL_INFOS() << "Cache purge took " << execute_time << " ms to execute for " << file_info.size() << " files" << LL_ENDL;
}
}
@ -236,89 +246,9 @@ const std::string LLDiskCache::assetTypeToString(LLAssetType::EType at)
return std::string("UNKNOWN");
}
const std::string LLDiskCache::metaDataToFilepath(const std::string id,
LLAssetType::EType at,
const std::string extra_info)
const std::string LLDiskCache::metaDataToFilepath(const std::string& id, LLAssetType::EType at)
{
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);
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), 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;
// 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, ec);
}
#else
// file last write time
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;
// 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, ec);
}
#endif
if (ec.failed())
{
LL_WARNS() << "Failed to update last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL;
}
return llformat("%s%s%s_%s_0.asset", sCacheDir.c_str(), gDirUtilp->getDirDelimiter().c_str(), CACHE_FILENAME_PREFIX.c_str(), id.c_str());
}
const std::string LLDiskCache::getCacheInfo()
@ -326,7 +256,7 @@ const std::string LLDiskCache::getCacheInfo()
std::ostringstream cache_info;
F32 max_in_mb = (F32)mMaxSizeBytes / (1024.0f * 1024.0f);
F32 percent_used = ((F32)dirFileSize(mCacheDir) / (F32)mMaxSizeBytes) * 100.0f;
F32 percent_used = ((F32)dirFileSize(sCacheDir) / (F32)mMaxSizeBytes) * 100.0f;
cache_info << std::fixed;
cache_info << std::setprecision(1);
@ -346,9 +276,9 @@ void LLDiskCache::clearCache()
*/
boost::system::error_code ec;
#if LL_WINDOWS
std::wstring cache_path(utf8str_to_utf16str(mCacheDir));
std::wstring cache_path(utf8str_to_utf16str(sCacheDir));
#else
std::string cache_path(mCacheDir);
std::string cache_path(sCacheDir);
#endif
if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed())
{
@ -357,7 +287,7 @@ void LLDiskCache::clearCache()
{
if (boost::filesystem::is_regular_file(*iter, ec) && !ec.failed())
{
if ((*iter).path().string().find(mCacheFilenamePrefix) != std::string::npos)
if ((*iter).path().string().find(CACHE_FILENAME_PREFIX) != std::string::npos)
{
boost::filesystem::remove(*iter, ec);
if (ec.failed())
@ -431,7 +361,7 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir)
{
if (boost::filesystem::is_regular_file(*iter, ec) && !ec.failed())
{
if ((*iter).path().string().find(mCacheFilenamePrefix) != std::string::npos)
if ((*iter).path().string().find(CACHE_FILENAME_PREFIX) != std::string::npos)
{
uintmax_t file_size = boost::filesystem::file_size(*iter, ec);
if (!ec.failed())

View File

@ -104,16 +104,9 @@ class LLDiskCache :
* 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);
static const std::string metaDataToFilepath(const std::string& id,
LLAssetType::EType at);
/**
* 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
@ -170,17 +163,7 @@ class LLDiskCache :
* 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;
static std::string sCacheDir;
/**
* When enabled, displays additional debugging information in

View File

@ -34,6 +34,8 @@
#include "llfasttimer.h"
#include "lldiskcache.h"
#include "boost/filesystem.hpp"
const S32 LLFileSystem::READ = 0x00000001;
const S32 LLFileSystem::WRITE = 0x00000002;
const S32 LLFileSystem::READ_WRITE = 0x00000003; // LLFileSystem::READ & LLFileSystem::WRITE
@ -56,9 +58,8 @@ LLFileSystem::LLFileSystem(const LLUUID& file_id, const LLAssetType::EType file_
{
// build the filename (TODO: we do this in a few places - perhaps we should factor into a single function)
std::string id;
mFileID.toString(id);
const std::string extra_info = "";
const std::string filename = LLDiskCache::getInstance()->metaDataToFilepath(id, mFileType, extra_info);
mFileID.asString();
const std::string filename = LLDiskCache::metaDataToFilepath(id, mFileType);
// update the last access time for the file if it exists - this is required
// even though we are reading and not writing because this is the
@ -67,7 +68,7 @@ LLFileSystem::LLFileSystem(const LLUUID& file_id, const LLAssetType::EType file_
bool exists = gDirUtilp->fileExists(filename);
if (exists)
{
LLDiskCache::getInstance()->updateFileAccessTime(filename);
updateFileAccessTime(filename);
}
}
}
@ -82,8 +83,7 @@ bool LLFileSystem::getExists(const LLUUID& file_id, const LLAssetType::EType fil
LL_PROFILE_ZONE_SCOPED;
std::string id_str;
file_id.toString(id_str);
const std::string extra_info = "";
const std::string filename = LLDiskCache::getInstance()->metaDataToFilepath(id_str, file_type, extra_info);
const std::string filename = LLDiskCache::metaDataToFilepath(id_str, file_type);
llifstream file(filename, std::ios::binary);
if (file.is_open())
@ -99,8 +99,7 @@ bool LLFileSystem::removeFile(const LLUUID& file_id, const LLAssetType::EType fi
{
std::string id_str;
file_id.toString(id_str);
const std::string extra_info = "";
const std::string filename = LLDiskCache::getInstance()->metaDataToFilepath(id_str, file_type, extra_info);
const std::string filename = LLDiskCache::metaDataToFilepath(id_str, file_type);
LLFile::remove(filename.c_str(), suppress_error);
@ -113,12 +112,11 @@ bool LLFileSystem::renameFile(const LLUUID& old_file_id, const LLAssetType::ETyp
{
std::string old_id_str;
old_file_id.toString(old_id_str);
const std::string extra_info = "";
const std::string old_filename = LLDiskCache::getInstance()->metaDataToFilepath(old_id_str, old_file_type, extra_info);
const std::string old_filename = LLDiskCache::metaDataToFilepath(old_id_str, old_file_type);
std::string new_id_str;
new_file_id.toString(new_id_str);
const std::string new_filename = LLDiskCache::getInstance()->metaDataToFilepath(new_id_str, new_file_type, extra_info);
const std::string new_filename = LLDiskCache::metaDataToFilepath(new_id_str, new_file_type);
// Rename needs the new file to not exist.
LLFileSystem::removeFile(new_file_id, new_file_type, ENOENT);
@ -140,8 +138,7 @@ S32 LLFileSystem::getFileSize(const LLUUID& file_id, const LLAssetType::EType fi
{
std::string id_str;
file_id.toString(id_str);
const std::string extra_info = "";
const std::string filename = LLDiskCache::getInstance()->metaDataToFilepath(id_str, file_type, extra_info);
const std::string filename = LLDiskCache::metaDataToFilepath(id_str, file_type);
S32 file_size = 0;
llifstream file(filename, std::ios::binary);
@ -160,8 +157,7 @@ bool LLFileSystem::read(U8* buffer, S32 bytes)
std::string id;
mFileID.toString(id);
const std::string extra_info = "";
const std::string filename = LLDiskCache::getInstance()->metaDataToFilepath(id, mFileType, extra_info);
const std::string filename = LLDiskCache::metaDataToFilepath(id, mFileType);
llifstream file(filename, std::ios::binary);
if (file.is_open())
@ -205,8 +201,7 @@ bool LLFileSystem::write(const U8* buffer, S32 bytes)
{
std::string id_str;
mFileID.toString(id_str);
const std::string extra_info = "";
const std::string filename = LLDiskCache::getInstance()->metaDataToFilepath(id_str, mFileType, extra_info);
const std::string filename = LLDiskCache::metaDataToFilepath(id_str, mFileType);
bool success = false;
@ -325,3 +320,64 @@ bool LLFileSystem::remove()
return true;
}
void LLFileSystem::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);
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), 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;
// 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, ec);
}
#else
// file last write time
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;
// 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, ec);
}
#endif
if (ec.failed())
{
LL_WARNS() << "Failed to update last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL;
}
}

View File

@ -53,6 +53,13 @@ class LLFileSystem
bool rename(const LLUUID& new_id, const LLAssetType::EType new_type);
bool remove();
/**
* 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);
static bool getExists(const LLUUID& file_id, const LLAssetType::EType file_type);
static bool removeFile(const LLUUID& file_id, const LLAssetType::EType file_type, int suppress_error = 0);
static bool renameFile(const LLUUID& old_file_id, const LLAssetType::EType old_file_type,