Skip to content

#5612 Reduce delays on updating file access #5615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
akleshchev wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

#5612 Reduce delays on updating file access #5615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 54 additions & 49 deletions indra/llfilesystem/llfilesystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "llfilesystem.h"
#include "llfasttimer.h"
#include "lldiskcache.h"
#include "workqueue.h"

#include "boost/filesystem.hpp"

Expand Down Expand Up @@ -290,63 +291,67 @@ bool LLFileSystem::remove() const
return true;
}

void LLFileSystem::updateFileAccessTime(const std::string& file_path)
void LLFileSystem::updateFileAccessTime(const std::string& file_path) const
{
/**
* 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
*/
constexpr 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(ll_convert<std::wstring>(file_path), ec);
if (ec.failed())
// Even a metadata file operation can take significant time if disk is busy.
// Don't hold main thread for this, schedule on a worker.
LL::WorkQueue::ptr_t workqueue = LL::WorkQueue::getInstance("General");
if (!workqueue)
{
LL_WARNS() << "Failed to read last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL;
return;
}
Comment on lines +298 to 302
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the "General" work queue doesn't exist yet, this returns silently and the access timestamp never gets updated. That changes behavior vs the previous synchronous implementation and can affect cache purging. Consider logging a warning and/or falling back to the synchronous timestamp update when the queue is unavailable (or using an existing filesystem thread facility).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@akleshchev akleshchev Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asset cache is normally relevant only once we are logged in, or at least are logging in. We do not update files during cleanup. But better safe than sorry, will do.


// 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)
// General queue maintains multiple threads, so there is a chance of a race
// condition, when multiple threads are trying to update the same file at
// the same time. This is not a problem because the last write time is
// only used to determine which files are old and some missed or
// extra updates will not cause any significant issues.
// last_write_time operation is supposed to be atomic, so multiple
// writes should not result in complications.
workqueue->post([file_path]()
{
Comment on lines +304 to 312
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using WorkQueue::post() can block the calling thread when the queue is at capacity (it waits until space is available). Since this method is called from the main thread specifically to avoid stalls, consider using tryPost() (and intentionally drop the update when full) or otherwise ensure the call cannot block; also check the returned bool so failures don’t silently skip updates.

Copilot uses AI. Check for mistakes.
boost::filesystem::last_write_time(ll_convert<std::wstring>(file_path), cur_time, ec);
}
/**
* 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...
*
* Expected cache lifetime is normally in days range.
* Let's start with 1 hour in time_t units and see how that unfolds
*/
constexpr std::time_t time_threshold = 1 * 60 * 60;

// current time
const std::time_t cur_time = std::time(nullptr);

Comment on lines +323 to +327
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cur_time is now computed inside the worker lambda, which means backlog/delay in the work queue increases (cur_time - last_write_time) and can cause extra timestamp writes that wouldn’t have happened at call time (defeating the 1h threshold’s purpose). To preserve prior semantics and minimize writes, compute cur_time before posting and capture it into the lambda.

Copilot uses AI. Check for mistakes.
boost::system::error_code ec;
#if LL_WINDOWS
const auto fs_path = ll_convert<std::wstring>(file_path);
#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;
}
const auto& fs_path = file_path;
#endif

// delta between cur time and last time the file was written
const std::time_t delta_time = cur_time - last_write_time;
// file last write time
const std::time_t last_write_time = boost::filesystem::last_write_time(fs_path, ec);
if (ec.failed())
{
LL_WARNS() << "Failed to read last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL;
return;
}

// 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
// delta between cur time and last time the file was written
const std::time_t delta_time = cur_time - last_write_time;

if (ec.failed())
{
LL_WARNS() << "Failed to update last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL;
}
// 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(fs_path, cur_time, ec);
if (ec.failed())
{
LL_WARNS() << "Failed to update last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL;
}
}
});
}
2 changes: 1 addition & 1 deletion indra/llfilesystem/llfilesystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class LLFileSystem
* 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);
void updateFileAccessTime(const std::string& file_path) const;

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);
Expand Down
Loading