Merge pull request #22665 from hrydgard/faster-firmware-install

Firmware install: Faster through parallelism

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.

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);
 

Core/Util/PSARUnpack.cpp

@@ -16,9 +16,15 @@
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
 #include <algorithm>
+#include <condition_variable>
 #include <cstring>
+#include <deque>
 #include <map>
+#include <memory>
+#include <mutex>
+#include <set>
 #include <string_view>
+#include <thread>
 
 #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<std::mutex> lock(mutex_);
+			quit_ = true;
+		}
+		cond_.notify_all();
+		for (std::thread &thread : threads_) {
+			thread.join();
+		}
+	}
+
+	void Write(const Path &destination, const std::vector<u8> &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<std::mutex> 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<std::mutex> 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<u8> data;
+	};
+
+	void ThreadFunc(int index) {
+		SetCurrentThreadName(StringFromFormat("PSARWrite %d", index).c_str());
+		// Content URIs are JNI calls.
+		AndroidJNIThreadContext jniContext;
+
+		std::unique_lock<std::mutex> 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<std::string> createdDirs_;
+	std::set<std::string> files_;
+	std::vector<std::thread> 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<Job> 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<int, std::map<std::string, std::string>> namesByModel;
+
+	std::unique_ptr<NewFileWriter> 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

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<void(float)> 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 {

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);