From c6c0d0bf42149116e2d9742a9fddb76d3ba3d378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 22 Jan 2024 15:04:21 +0100 Subject: [PATCH] Android memstick folder moves: Do recursive moves if in the same device. Drastically faster. --- Common/File/FileUtil.cpp | 37 ++++++++++---------------- Common/File/FileUtil.h | 2 +- Core/Util/GameManager.cpp | 1 + Core/Util/MemStick.cpp | 48 +++++++++++++++++++++++++++++----- Core/Util/MemStick.h | 2 +- GPU/Common/TextureReplacer.cpp | 4 ++- UI/MemStickScreen.cpp | 14 +++++++--- 7 files changed, 72 insertions(+), 36 deletions(-) diff --git a/Common/File/FileUtil.cpp b/Common/File/FileUtil.cpp index 2959056aed6b..a2ac17f9c980 100644 --- a/Common/File/FileUtil.cpp +++ b/Common/File/FileUtil.cpp @@ -628,13 +628,12 @@ bool Rename(const Path &srcFilename, const Path &destFilename) { break; case PathType::CONTENT_URI: // Content URI: Can only rename if in the same folder. - // TODO: Fallback to move + rename? Or do we even care about that use case? + // TODO: Fallback to move + rename? Or do we even care about that use case? We have MoveIfFast for such tricks. if (srcFilename.GetDirectory() != destFilename.GetDirectory()) { INFO_LOG(COMMON, "Content URI rename: Directories not matching, failing. %s --> %s", srcFilename.c_str(), destFilename.c_str()); return false; } INFO_LOG(COMMON, "Content URI rename: %s --> %s", srcFilename.c_str(), destFilename.c_str()); - return Android_RenameFileTo(srcFilename.ToString(), destFilename.GetFilename()) == StorageError::SUCCESS; default: return false; @@ -752,22 +751,12 @@ bool Copy(const Path &srcFilename, const Path &destFilename) { // Will overwrite the target. bool Move(const Path &srcFilename, const Path &destFilename) { - // Try a shortcut in Android Storage scenarios. - if (srcFilename.Type() == PathType::CONTENT_URI && destFilename.Type() == PathType::CONTENT_URI && srcFilename.CanNavigateUp() && destFilename.CanNavigateUp()) { - // We do not handle simultaneous renames here. - if (srcFilename.GetFilename() == destFilename.GetFilename()) { - Path srcParent = srcFilename.NavigateUp(); - Path dstParent = destFilename.NavigateUp(); - if (Android_MoveFile(srcFilename.ToString(), srcParent.ToString(), dstParent.ToString()) == StorageError::SUCCESS) { - return true; - } - // If failed, fall through and try other ways. - } - } - - if (Rename(srcFilename, destFilename)) { + bool fast = MoveIfFast(srcFilename, destFilename); + if (fast) { return true; - } else if (Copy(srcFilename, destFilename)) { + } + // OK, that failed, so fall back on a copy. + if (Copy(srcFilename, destFilename)) { return Delete(srcFilename); } else { return false; @@ -775,7 +764,13 @@ bool Move(const Path &srcFilename, const Path &destFilename) { } bool MoveIfFast(const Path &srcFilename, const Path &destFilename) { - if (srcFilename.Type() == PathType::CONTENT_URI && destFilename.Type() == PathType::CONTENT_URI && srcFilename.CanNavigateUp() && destFilename.CanNavigateUp()) { + if (srcFilename.Type() != destFilename.Type()) { + // No way it's gonna work. + return false; + } + + // Only need to check one type here, due to the above check. + if (srcFilename.Type() == PathType::CONTENT_URI && srcFilename.CanNavigateUp() && destFilename.CanNavigateUp()) { if (srcFilename.GetFilename() == destFilename.GetFilename()) { Path srcParent = srcFilename.NavigateUp(); Path dstParent = destFilename.NavigateUp(); @@ -787,11 +782,7 @@ bool MoveIfFast(const Path &srcFilename, const Path &destFilename) { } } - if (srcFilename.Type() != destFilename.Type()) { - // No way it's gonna work. - return false; - } - + // Try a traditional rename operation. return Rename(srcFilename, destFilename); } diff --git a/Common/File/FileUtil.h b/Common/File/FileUtil.h index 2ae3329b753b..5b7b779e8ebf 100644 --- a/Common/File/FileUtil.h +++ b/Common/File/FileUtil.h @@ -97,7 +97,7 @@ bool DeleteDir(const Path &filename); // Deletes the given directory and anything under it. Returns true on success. bool DeleteDirRecursively(const Path &directory); -// Renames file srcFilename to destFilename, returns true on success +// Renames/moves file srcFilename to destFilename, returns true on success // Will usually only work with in the same partition or other unit of storage, // so you might have to fall back to copy/delete. bool Rename(const Path &srcFilename, const Path &destFilename); diff --git a/Core/Util/GameManager.cpp b/Core/Util/GameManager.cpp index 628f2d29fa81..6693c4cdb0fa 100644 --- a/Core/Util/GameManager.cpp +++ b/Core/Util/GameManager.cpp @@ -345,6 +345,7 @@ bool GameManager::InstallGame(const Path &url, const Path &fileName, bool delete if (info.stripChars == 0) { success = InstallMemstickZip(z, fileName, dest / "textures.zip", info, deleteAfter); } else { + // TODO: Can probably remove this, as we now put .nomedia in /TEXTURES directly. File::CreateEmptyFile(dest / ".nomedia"); success = InstallMemstickGame(z, fileName, dest, info, true, deleteAfter); } diff --git a/Core/Util/MemStick.cpp b/Core/Util/MemStick.cpp index aa27a2b2eea6..3e0eba41e1ea 100644 --- a/Core/Util/MemStick.cpp +++ b/Core/Util/MemStick.cpp @@ -62,7 +62,6 @@ bool SwitchMemstickFolderTo(Path newMemstickFolder) { return true; } - // Keep the size with the file, so we can skip overly large ones in the move. // The user will have to take care of them afterwards, it'll just take too long probably. struct FileSuffix { @@ -100,6 +99,28 @@ static bool ListFileSuffixesRecursively(const Path &root, const Path &folder, st return true; } +bool MoveChildrenFast(const Path &moveSrc, const Path &moveDest, MoveProgressReporter &progressReporter) { + std::vector files; + progressReporter.SetStatus("Starting move..."); + if (!File::GetFilesInDir(moveSrc, &files)) { + return false; + } + + for (auto file : files) { + // Construct destination path + Path fileSrc = file.fullName; + Path fileDest = moveDest / file.name; + progressReporter.SetStatus(file.name); + INFO_LOG(SYSTEM, "About to move PSP data from '%s' to '%s'", fileSrc.c_str(), fileDest.c_str()); + bool result = File::MoveIfFast(fileSrc, fileDest); + if (!result) { + // TODO: Should we try to move back anything that succeeded before this one? + return false; + } + } + return true; +} + MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressReporter &progressReporter) { auto ms = GetI18NCategory(I18NCat::MEMSTICK); if (moveSrc.GetFilename() != "PSP") { @@ -112,6 +133,20 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR INFO_LOG(SYSTEM, "About to move PSP data from '%s' to '%s'", moveSrc.c_str(), moveDest.c_str()); + // First, we try the cheapest and safest way to move: Can we move files directly within the same device? + // We loop through the files/dirs in the source directory and just try to move them, it should work. + if (MoveChildrenFast(moveSrc, moveDest, progressReporter)) { + INFO_LOG(SYSTEM, "Quick-move succeeded"); + progressReporter.SetStatus(ms->T("Done!")); + return new MoveResult{ + true, "" + }; + } + + // If this doesn't work, we'll fall back on a recursive *copy* (disk space is less of a concern when + // moving from device to device, other than that everything fits on the destination). + // Then we verify the results before we delete the originals. + // Search through recursively, listing the files to move and also summing their sizes. std::vector fileSuffixesToMove; std::vector directorySuffixesToCreate; @@ -121,7 +156,7 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR // TODO: Handle failure listing files. std::string error = "Failed to read old directory"; INFO_LOG(SYSTEM, "%s", error.c_str()); - progressReporter.Set(ms->T(error.c_str())); + progressReporter.SetStatus(ms->T(error.c_str())); return new MoveResult{ false, error }; } @@ -144,14 +179,14 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR INFO_LOG(SYSTEM, "dry run: Would have created dir '%s'", dir.c_str()); } else { INFO_LOG(SYSTEM, "Creating dir '%s'", dir.c_str()); - progressReporter.Set(dirSuffix); + progressReporter.SetStatus(dirSuffix); // Just ignore already-exists errors. File::CreateDir(dir); } } for (auto &fileSuffix : fileSuffixesToMove) { - progressReporter.Set(StringFromFormat("%s (%s)", fileSuffix.suffix.c_str(), NiceSizeFormat(fileSuffix.fileSize).c_str())); + progressReporter.SetStatus(StringFromFormat("%s (%s)", fileSuffix.suffix.c_str(), NiceSizeFormat(fileSuffix.fileSize).c_str())); Path from = moveSrc / fileSuffix.suffix; Path to = moveDest / fileSuffix.suffix; @@ -163,8 +198,7 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR } else { if (!File::MoveIfFast(from, to)) { INFO_LOG(SYSTEM, "Skipped moving file '%s' to '%s' (%s)", from.c_str(), to.c_str(), NiceSizeFormat(fileSuffix.fileSize).c_str()); - skippedFiles++; - } else { + skippedFiles++; INFO_LOG(SYSTEM, "Moved file '%s' to '%s'", from.c_str(), to.c_str()); } } @@ -193,7 +227,7 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR INFO_LOG(SYSTEM, "dry run: Would have deleted dir '%s'", dir.c_str()); } else { INFO_LOG(SYSTEM, "Deleting dir '%s'", dir.c_str()); - progressReporter.Set(dirSuffix); + progressReporter.SetStatus(dirSuffix); if (File::Exists(dir)) { File::DeleteDir(dir); } diff --git a/Core/Util/MemStick.h b/Core/Util/MemStick.h index 6fdb868768dc..cf71d806ce02 100644 --- a/Core/Util/MemStick.h +++ b/Core/Util/MemStick.h @@ -8,7 +8,7 @@ class MoveProgressReporter { public: - void Set(const std::string &value) { + void SetStatus(const std::string &value) { std::lock_guard guard(mutex_); progress_ = value; } diff --git a/GPU/Common/TextureReplacer.cpp b/GPU/Common/TextureReplacer.cpp index d85e8372213b..9ed53c7cd8c6 100644 --- a/GPU/Common/TextureReplacer.cpp +++ b/GPU/Common/TextureReplacer.cpp @@ -87,8 +87,10 @@ void TextureReplacer::NotifyConfigChanged() { // If we're saving, auto-create the directory. if (g_Config.bSaveNewTextures && !File::Exists(newTextureDir_)) { + INFO_LOG(G3D, "Creating new texture directory: '%s'", newTextureDir_.ToVisualString().c_str()); File::CreateFullPath(newTextureDir_); - File::CreateEmptyFile(newTextureDir_ / ".nomedia"); + // We no longer create a nomedia file here, since we put one + // in the TEXTURES root. } enabled_ = File::IsDirectory(basePath_); diff --git a/UI/MemStickScreen.cpp b/UI/MemStickScreen.cpp index 8e54b8d4ca50..b528db0b7026 100644 --- a/UI/MemStickScreen.cpp +++ b/UI/MemStickScreen.cpp @@ -552,12 +552,12 @@ void ConfirmMemstickMoveScreen::update() { if (result) { if (result->success) { - progressReporter_.Set(iz->T("Done!")); + progressReporter_.SetStatus(iz->T("Done!")); INFO_LOG(SYSTEM, "Move data task finished successfully!"); // Succeeded! FinishFolderMove(); } else { - progressReporter_.Set(iz->T("Failed to move some files!")); + progressReporter_.SetStatus(iz->T("Failed to move some files!")); INFO_LOG(SYSTEM, "Move data task failed!"); // What do we do here? We might be in the middle of a move... Bad. RecreateViews(); @@ -575,7 +575,7 @@ UI::EventReturn ConfirmMemstickMoveScreen::OnConfirm(UI::EventParams ¶ms) { // If the directory itself is called PSP, don't go below. if (moveData_) { - progressReporter_.Set(T(I18NCat::MEMSTICK, "Starting move...")); + progressReporter_.SetStatus(T(I18NCat::MEMSTICK, "Starting move...")); moveDataTask_ = Promise::Spawn(&g_threadManager, [&]() -> MoveResult * { Path moveSrc = g_Config.memStickDirectory; @@ -594,6 +594,8 @@ UI::EventReturn ConfirmMemstickMoveScreen::OnConfirm(UI::EventParams ¶ms) { void ConfirmMemstickMoveScreen::FinishFolderMove() { auto ms = GetI18NCategory(I18NCat::MEMSTICK); + Path oldMemstickFolder = g_Config.memStickDirectory; + // Successful so far, switch the memstick folder. if (!SwitchMemstickFolderTo(newMemstickFolder_)) { // TODO: More precise errors. @@ -603,6 +605,12 @@ void ConfirmMemstickMoveScreen::FinishFolderMove() { // If the chosen folder already had a config, reload it! g_Config.Load(); + + // If the current browser directory is the old memstick folder, drop it. + if (g_Config.currentDirectory == oldMemstickFolder) { + g_Config.currentDirectory = g_Config.defaultCurrentDirectory; + } + PostLoadConfig(); if (!initialSetup_) {