From 9ddfcca6702eb5a44c2bda38ddc364c9fa4e9015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 14 Oct 2026 19:27:41 +0200 Subject: [PATCH] Firmware install: Fewer and parallel file operations into the staging directory Through Android's storage framework, every exists check, create and open is a round trip to the storage provider, and writing each file took four of them (a check of its directory, a check of the file, the create, the open). That made a 6.61 install take around 30 s on a phone, against a fraction of a second of decrypting and inflating. The install unpacks into a staging directory it has just cleared, so nothing in it can exist yet. With the new outputDirIsEmpty option, each directory is created once and each file is created and opened without checking for it first (File::WriteDataToNewFile), on the IO threads while decoding goes on. A name that turns up twice still goes the careful way, overwriting the first. The swap into place at the end is unchanged. Headless --unpack-updater uses the same path when the output directory doesn't exist yet. Its output is identical either way. With SIMULATE_SLOW_IO, unpacking the fonts went from 17 s to 7.5 s, most of what's left being the fixed cost of opening the updater. The UI's install already runs on a thread pool IO thread, so writes queued on the same pool could stall behind other long IO tasks, and the pool drops queued tasks unrun at teardown, which would have left the install waiting forever on its in-flight count. Eight threads owned by the writer (attached for JNI) avoid both, and headless no longer has to start the pool for it. Also compare file names case-insensitively when looking for repeats, since the storage usually is case-insensitive and a second create there makes a renamed copy. Checked with ThreadSanitizer, and on a read-only destination (every file fails, no hang). Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/File/FileUtil.cpp | 32 +++++++++ Common/File/FileUtil.h | 4 ++ Core/Util/PSARUnpack.cpp | 148 ++++++++++++++++++++++++++++++++++++++- Core/Util/PSARUnpack.h | 4 ++ headless/Headless.cpp | 2 + 5 files changed, 188 insertions(+), 2 deletions(-) diff --git a/Common/File/FileUtil.cpp b/Common/File/FileUtil.cpp index 09b7f9d83d..f5da3c83c6 100644 --- a/Common/File/FileUtil.cpp +++ b/Common/File/FileUtil.cpp @@ -1549,6 +1549,38 @@ bool WriteDataToFile(bool text_file, const void* data, size_t size, const Path & return true; } +bool WriteDataToNewFile(const void *data, size_t size, const Path &filename) { +#if PPSSPP_PLATFORM(ANDROID) && !defined(HAVE_LIBRETRO_VFS) + if (filename.Type() == PathType::CONTENT_URI) { + if (!filename.CanNavigateUp() || Android_CreateFile(filename.NavigateUp().ToString(), filename.GetFilename()) != StorageError::SUCCESS) { + return false; + } + const int fd = Android_OpenContentUriFd(filename.ToString(), Android_OpenContentUriMode::READ_WRITE_TRUNCATE); + if (fd < 0) { + return false; + } + const u8 *p = (const u8 *)data; + size_t left = size; + int interrupts = 0; + while (left > 0) { + const ssize_t written = write(fd, p, left); + if (written < 0 && errno == EINTR && ++interrupts < 100) { + continue; + } + if (written <= 0) { + close(fd); + return false; + } + p += written; + left -= written; + interrupts = 0; + } + return close(fd) == 0; + } +#endif + return WriteDataToFile(false, data, size, filename); +} + void ChangeMTime(const Path &path, time_t mtime) { if (path.Type() == PathType::CONTENT_URI) { // No clue what to do here. There doesn't seem to be a way. diff --git a/Common/File/FileUtil.h b/Common/File/FileUtil.h index 23573c1911..0b66d4c898 100644 --- a/Common/File/FileUtil.h +++ b/Common/File/FileUtil.h @@ -244,6 +244,10 @@ void InitLibretroVFS(const struct retro_vfs_interface_info *vfs) noexcept; // Whole-file reading/writing bool WriteStringToFile(bool textFile, std::string_view str, const Path &filename); bool WriteDataToFile(bool textFile, const void* data, size_t size, const Path &filename); +// For a file that's known not to exist yet. On Android content URIs, this saves the existence +// check WriteDataToFile makes, a round trip to the storage provider - but if the file does exist, +// the provider makes a renamed copy next to it instead of overwriting it. +bool WriteDataToNewFile(const void *data, size_t size, const Path &filename); bool ReadFileToStringOptions(bool textFile, bool allowShort, const Path &path, std::string *str); diff --git a/Core/Util/PSARUnpack.cpp b/Core/Util/PSARUnpack.cpp index 9efbfae9d7..15d5d758a8 100644 --- a/Core/Util/PSARUnpack.cpp +++ b/Core/Util/PSARUnpack.cpp @@ -16,9 +16,15 @@ // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. #include +#include #include +#include #include +#include +#include +#include #include +#include #include "zlib.h" @@ -29,6 +35,7 @@ #include "Common/Log.h" #include "Common/StringUtils.h" #include "Common/System/OSD.h" +#include "Common/Thread/ThreadUtil.h" #include "Core/Config.h" #include "Core/ELF/ParamSFO.h" #include "Core/ELF/PBPReader.h" @@ -731,6 +738,128 @@ void PSARReader::EnsureContents() { // Anything else we leave empty - the caller reports it through the stats. } +// Writes unpacked files into a directory that started out empty, on threads of its own while the +// caller goes on decoding. Each directory is created once, and files without checking whether +// they exist. Through Android's storage framework those checks and creates are round trips to +// another process, which is nearly all of an install's time, and doing several at once hides most +// of the wait. The threads are its own rather than the thread pool's: the UI's install already +// runs on a pool thread, and pool tasks can be dropped unrun at teardown, which would leave +// Finish() waiting forever. +class NewFileWriter { +public: + explicit NewFileWriter(PSARUnpackStats *stats) : stats_(stats) { + for (int i = 0; i < NUM_THREADS; i++) { + threads_.emplace_back([this, i]() { ThreadFunc(i); }); + } + } + ~NewFileWriter() { + Finish(); + { + std::lock_guard lock(mutex_); + quit_ = true; + } + cond_.notify_all(); + for (std::thread &thread : threads_) { + thread.join(); + } + } + + void Write(const Path &destination, const std::vector &data) { + const Path dir = destination.NavigateUp(); + if (createdDirs_.insert(dir.ToString()).second && !File::CreateFullPath(dir)) { + ERROR_LOG(Log::Loader, "PSAR: couldn't create a directory for '%s'", destination.c_str()); + createdDirs_.erase(dir.ToString()); + stats_->failed++; + return; + } + + // Case insensitive, since the storage usually is. + std::string key = destination.ToString(); + std::transform(key.begin(), key.end(), key.begin(), [](unsigned char c) { return (char)tolower(c); }); + if (!files_.insert(key).second) { + // The same name twice. The later one wins, as it always has, so let the first land + // and then overwrite it the careful way - a blind create would make a renamed copy. + Finish(); + if (File::WriteDataToFile(false, data.data(), data.size(), destination)) { + stats_->written++; + } else { + ERROR_LOG(Log::Loader, "PSAR: couldn't write '%s'", destination.c_str()); + stats_->failed++; + } + return; + } + + std::unique_lock lock(mutex_); + // Bounded, so memory doesn't fill up with decoded files waiting for a slow disk. + cond_.wait(lock, [&] { return queue_.size() + busy_ < MAX_IN_FLIGHT; }); + queue_.push_back({ destination, data }); + cond_.notify_all(); + } + + // Waits for every write so far, and adds their results to the stats. + void Finish() { + std::unique_lock lock(mutex_); + cond_.wait(lock, [&] { return queue_.empty() && busy_ == 0; }); + stats_->written += written_; + stats_->failed += failed_; + written_ = 0; + failed_ = 0; + } + +private: + struct Job { + Path destination; + std::vector data; + }; + + void ThreadFunc(int index) { + SetCurrentThreadName(StringFromFormat("PSARWrite %d", index).c_str()); + // Content URIs are JNI calls. + AndroidJNIThreadContext jniContext; + + std::unique_lock lock(mutex_); + while (true) { + cond_.wait(lock, [&] { return quit_ || !queue_.empty(); }); + if (queue_.empty()) { + return; + } + Job job = std::move(queue_.front()); + queue_.pop_front(); + busy_++; + lock.unlock(); + + const bool ok = File::WriteDataToNewFile(job.data.data(), job.data.size(), job.destination); + if (!ok) { + ERROR_LOG(Log::Loader, "PSAR: couldn't write '%s'", job.destination.c_str()); + } + + lock.lock(); + busy_--; + if (ok) + written_++; + else + failed_++; + cond_.notify_all(); + } + } + + static constexpr int NUM_THREADS = 8; + static constexpr size_t MAX_IN_FLIGHT = 8; + + PSARUnpackStats *stats_; + std::set createdDirs_; + std::set files_; + std::vector threads_; + // The rest are guarded by mutex_. One condition variable covers all the waits; they're short. + std::mutex mutex_; + std::condition_variable cond_; + std::deque queue_; + size_t busy_ = 0; + int written_ = 0; + int failed_ = 0; + bool quit_ = false; +}; + bool UnpackPSAR(const u8 *psar, size_t psarSize, const Path &outputDir, const PSARUnpackOptions &options, PSARUnpackStats *stats, std::string *error) { PSARUnpackStats localStats; if (!stats) { @@ -751,6 +880,11 @@ bool UnpackPSAR(const u8 *psar, size_t psarSize, const Path &outputDir, const PS // One file list per model, filled in as we go - they come before the files they name. std::map> namesByModel; + + std::unique_ptr newFileWriter; + if (options.outputDirIsEmpty && !options.listOnly) { + newFileWriter.reset(new NewFileWriter(stats)); + } INFO_LOG(Log::Loader, "Resolving names against model %s", PSPModelGenerationToString(options.model)); while (true) { @@ -893,6 +1027,10 @@ bool UnpackPSAR(const u8 *psar, size_t psarSize, const Path &outputDir, const PS } const Path destination = outputDir / relative; + if (newFileWriter) { + newFileWriter->Write(destination, reader.entryData()); + continue; + } if (!File::CreateFullPath(destination.NavigateUp())) { ERROR_LOG(Log::Loader, "PSAR: couldn't create a directory for '%s'", relative.c_str()); stats->failed++; @@ -906,6 +1044,9 @@ bool UnpackPSAR(const u8 *psar, size_t psarSize, const Path &outputDir, const PS stats->written++; } + if (newFileWriter) { + newFileWriter->Finish(); + } return true; } @@ -1440,9 +1581,12 @@ bool InstallFirmware(const Path &updater, const Path &nandRoot, const PSARUnpack return false; } + // The staging directory was just cleared, so the unpack can skip checking for each file. + PSARUnpackOptions stagingOptions = options; + stagingOptions.outputDirIsEmpty = true; bool ok = updater.empty() - ? UnpackUpdaterFromMountedDisc(staging, options, stats, error) - : UnpackUpdater(updater, staging, options, stats, error); + ? UnpackUpdaterFromMountedDisc(staging, stagingOptions, stats, error) + : UnpackUpdater(updater, staging, stagingOptions, stats, error); if (ok && stats->written == 0) { // Nothing came out, so the archive had no file list for the model we asked for - old diff --git a/Core/Util/PSARUnpack.h b/Core/Util/PSARUnpack.h index b448938c32..9af8392c9d 100644 --- a/Core/Util/PSARUnpack.h +++ b/Core/Util/PSARUnpack.h @@ -89,6 +89,10 @@ struct PSARUnpackOptions { // Called on the unpacking thread after every entry, with how far through the archive we are // (0.0 to 1.0). Unpacking a full firmware takes a while, so a UI wants this. std::function progress; + // outputDir was just created and is empty, so nothing in it can exist yet. Files are then + // created without checking for them first, and written on several threads. Through Android's + // storage framework every check and create is a slow round trip, so this saves most of the time. + bool outputDirIsEmpty = false; }; struct PSARUnpackStats { diff --git a/headless/Headless.cpp b/headless/Headless.cpp index e5588c146f..7dd2871a7a 100644 --- a/headless/Headless.cpp +++ b/headless/Headless.cpp @@ -920,6 +920,8 @@ int main(int argc, const char* argv[]) { fprintf(stderr, "Unknown PSP model '%s' - expected 01g..12g or any\n", cmdLineOptions.unpackUpdaterModel.value().c_str()); return 1; } + // A directory that isn't there yet will be empty, so the unpack can skip checking for each file. + unpackOptions.outputDirIsEmpty = !File::Exists(Path(cmdLineOptions.unpackUpdater.value())); PSARUnpackStats stats; std::string unpackError; const bool ok = UnpackUpdater(Path(cmdLineOptions.bootFilenames[0]), Path(cmdLineOptions.unpackUpdater.value()), unpackOptions, &stats, &unpackError);