Skip to content

Commit

Permalink
Merge pull request #19671 from hrydgard/file-system-perf-part-2
Browse files Browse the repository at this point in the history
File system perf, part 2
  • Loading branch information
hrydgard authored Nov 29, 2024
2 parents b15f7fd + 720e056 commit 398b181
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 42 deletions.
17 changes: 9 additions & 8 deletions Common/File/AndroidStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void Android_StorageSetNativeActivity(jobject nativeActivity) {
void Android_RegisterStorageCallbacks(JNIEnv * env, jobject obj) {
openContentUri = env->GetMethodID(env->GetObjectClass(obj), "openContentUri", "(Ljava/lang/String;Ljava/lang/String;)I");
_dbg_assert_(openContentUri);
listContentUriDir = env->GetMethodID(env->GetObjectClass(obj), "listContentUriDir", "(Ljava/lang/String;)[Ljava/lang/String;");
listContentUriDir = env->GetMethodID(env->GetObjectClass(obj), "listContentUriDir", "(Ljava/lang/String;Ljava/lang/String;)[Ljava/lang/String;");
_dbg_assert_(listContentUriDir);
contentUriCreateDirectory = env->GetMethodID(env->GetObjectClass(obj), "contentUriCreateDirectory", "(Ljava/lang/String;Ljava/lang/String;)I");
_dbg_assert_(contentUriCreateDirectory);
Expand Down Expand Up @@ -222,18 +222,19 @@ bool Android_FileExists(const std::string &fileUri) {
return exists;
}

std::vector<File::FileInfo> Android_ListContentUri(const std::string &path, bool *exists) {
std::vector<File::FileInfo> Android_ListContentUri(const std::string &uri, const std::string &prefix, bool *exists) {
if (!g_nativeActivity) {
*exists = false;
return std::vector<File::FileInfo>();
return {};
}
auto env = getEnv();
*exists = true;

double start = time_now_d();

jstring param = env->NewStringUTF(path.c_str());
jobject retval = env->CallObjectMethod(g_nativeActivity, listContentUriDir, param);
jstring param = env->NewStringUTF(uri.c_str());
jstring filter = prefix.empty() ? nullptr : env->NewStringUTF(prefix.c_str());
jobject retval = env->CallObjectMethod(g_nativeActivity, listContentUriDir, param, filter);

jobjectArray fileList = (jobjectArray)retval;
std::vector<File::FileInfo> items;
Expand All @@ -245,11 +246,11 @@ std::vector<File::FileInfo> Android_ListContentUri(const std::string &path, bool
std::string line = charArray;
File::FileInfo info{};
if (line == "X") {
// Indicates an exception thrown, path doesn't exist.
// Indicates an exception thrown, uri doesn't exist.
*exists = false;
} else if (ParseFileInfo(line, &info)) {
// We can just reconstruct the URI.
info.fullName = Path(path) / info.name;
info.fullName = Path(uri) / info.name;
items.push_back(info);
}
}
Expand All @@ -261,7 +262,7 @@ std::vector<File::FileInfo> Android_ListContentUri(const std::string &path, bool
double elapsed = time_now_d() - start;
double threshold = 0.1;
if (elapsed >= threshold) {
INFO_LOG(Log::FileSystem, "Listing directory on content URI '%s' took %0.3f s (%d files, log threshold = %0.3f)", path.c_str(), elapsed, (int)items.size(), threshold);
INFO_LOG(Log::FileSystem, "Listing directory on content URI '%s' took %0.3f s (%d files, log threshold = %0.3f)", uri.c_str(), elapsed, (int)items.size(), threshold);
}
return items;
}
Expand Down
6 changes: 4 additions & 2 deletions Common/File/AndroidStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ extern std::string g_extFilesDir;
extern std::string g_externalDir;
extern std::string g_nativeLibDir;

// Note that we don't use string_view much here because NewStringUTF doesn't have a size parameter.

#if PPSSPP_PLATFORM(ANDROID) && !defined(__LIBRETRO__)

#include <jni.h>
Expand All @@ -60,7 +62,7 @@ int64_t Android_GetFreeSpaceByFilePath(const std::string &filePath);
bool Android_IsExternalStoragePreservedLegacy();
const char *Android_ErrorToString(StorageError error);

std::vector<File::FileInfo> Android_ListContentUri(const std::string &uri, bool *exists);
std::vector<File::FileInfo> Android_ListContentUri(const std::string &uri, const std::string &prefix, bool *exists);

void Android_RegisterStorageCallbacks(JNIEnv * env, jobject obj);

Expand All @@ -85,7 +87,7 @@ inline int64_t Android_GetFreeSpaceByContentUri(const std::string &uri) { return
inline int64_t Android_GetFreeSpaceByFilePath(const std::string &filePath) { return -1; }
inline bool Android_IsExternalStoragePreservedLegacy() { return false; }
inline const char *Android_ErrorToString(StorageError error) { return ""; }
inline std::vector<File::FileInfo> Android_ListContentUri(const std::string &uri, bool *exists) {
inline std::vector<File::FileInfo> Android_ListContentUri(const std::string &uri, const std::string &prefix, bool *exists) {
*exists = false;
return std::vector<File::FileInfo>();
}
Expand Down
43 changes: 29 additions & 14 deletions Common/File/DirListing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,43 +153,47 @@ bool FileInfo::operator <(const FileInfo & other) const {
return false;
}

std::vector<File::FileInfo> ApplyFilter(std::vector<File::FileInfo> files, const char *filter) {
std::vector<File::FileInfo> ApplyFilter(std::vector<File::FileInfo> files, const char *extensionFilter, std::string_view prefix) {
std::set<std::string> filters;
if (filter) {
if (extensionFilter) {
std::string tmp;
while (*filter) {
if (*filter == ':') {
while (*extensionFilter) {
if (*extensionFilter == ':') {
filters.emplace("." + tmp);
tmp.clear();
} else {
tmp.push_back(*filter);
tmp.push_back(*extensionFilter);
}
filter++;
extensionFilter++;
}
if (!tmp.empty())
filters.emplace("." + tmp);
}

auto pred = [&](const File::FileInfo &info) {
if (info.isDirectory || !filter)
if (info.isDirectory || !extensionFilter)
return false;
std::string ext = info.fullName.GetFileExtension();
if (!startsWith(info.name, prefix)) {
return false;
}
return filters.find(ext) == filters.end();
};
files.erase(std::remove_if(files.begin(), files.end(), pred), files.end());
return files;
}

bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const char *filter, int flags) {
bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const char *filter, int flags, std::string_view prefix) {
if (SIMULATE_SLOW_IO) {
INFO_LOG(Log::System, "GetFilesInDir %s", directory.c_str());
INFO_LOG(Log::System, "GetFilesInDir %s (ext %s, prefix %.*s)", directory.c_str(), filter, (int)prefix.size(), prefix.data());
sleep_ms(300, "slow-io-sim");
}

if (directory.Type() == PathType::CONTENT_URI) {
bool exists = false;
std::vector<File::FileInfo> fileList = Android_ListContentUri(directory.ToString(), &exists);
*files = ApplyFilter(fileList, filter);
// TODO: Move prefix filtering over to the Java side for more speed.
std::vector<File::FileInfo> fileList = Android_ListContentUri(directory.ToString(), std::string(prefix), &exists);
*files = ApplyFilter(fileList, filter, "");
std::sort(files->begin(), files->end());
return exists;
}
Expand All @@ -213,6 +217,7 @@ bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const ch
#if PPSSPP_PLATFORM(WINDOWS)
if (directory.IsRoot()) {
// Special path that means root of file system.
// This does not respect prefix filtering.
std::vector<std::string> drives = File::GetWindowsDrives();
for (auto drive = drives.begin(); drive != drives.end(); ++drive) {
if (*drive == "A:/" || *drive == "B:/")
Expand Down Expand Up @@ -248,9 +253,6 @@ bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const ch
return false;
}
do {
if (SIMULATE_SLOW_IO) {
sleep_ms(100, "slow-io-sim");
}
const std::string virtualName = ConvertWStringToUTF8(ffd.cFileName);
// check for "." and ".."
if (!(flags & GETFILES_GET_NAVIGATION_ENTRIES) && (virtualName == "." || virtualName == ".."))
Expand All @@ -261,6 +263,15 @@ bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const ch
continue;
}

if (!startsWith(virtualName, prefix)) {
continue;
}

if (SIMULATE_SLOW_IO) {
INFO_LOG(Log::System, "GetFilesInDir item %s", virtualName.c_str());
sleep_ms(50, "slow-io-sim");
}

FileInfo info;
info.name = virtualName;
info.fullName = directory / virtualName;
Expand Down Expand Up @@ -308,6 +319,10 @@ bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const ch
continue;
}

if (!startsWith(virtualName, prefix)) {
continue;
}

// Let's just reuse GetFileInfo. We're calling stat anyway to get isDirectory information.
Path fullName = directory / virtualName;

Expand Down
5 changes: 3 additions & 2 deletions Common/File/DirListing.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include <string>
#include <string_view>
#include <vector>
#include <cstdio>
#include <cstdint>
Expand Down Expand Up @@ -32,8 +33,8 @@ enum {
GETFILES_GET_NAVIGATION_ENTRIES = 2, // If you don't set this, "." and ".." will be skipped.
};

bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const char *filter = nullptr, int flags = 0);
std::vector<File::FileInfo> ApplyFilter(std::vector<File::FileInfo> files, const char *filter);
bool GetFilesInDir(const Path &directory, std::vector<FileInfo> *files, const char *extensionFilter = nullptr, int flags = 0, std::string_view prefix = std::string_view());
std::vector<File::FileInfo> ApplyFilter(std::vector<File::FileInfo> files, const char *extensionFilter, std::string_view prefix);

#ifdef _WIN32
std::vector<std::string> GetWindowsDrives();
Expand Down
4 changes: 2 additions & 2 deletions Common/File/PathBrowser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ std::string PathBrowser::GetFriendlyPath() const {
return path_.ToVisualString();
}

bool PathBrowser::GetListing(std::vector<File::FileInfo> &fileInfo, const char *filter, bool *cancel) {
bool PathBrowser::GetListing(std::vector<File::FileInfo> &fileInfo, const char *extensionFilter, bool *cancel) {
std::unique_lock<std::mutex> guard(pendingLock_);
while (!IsListingReady() && (!cancel || !*cancel)) {
// In case cancel changes, just sleep. TODO: Replace with condition variable.
Expand All @@ -239,7 +239,7 @@ bool PathBrowser::GetListing(std::vector<File::FileInfo> &fileInfo, const char *
guard.lock();
}

fileInfo = ApplyFilter(pendingFiles_, filter);
fileInfo = ApplyFilter(pendingFiles_, extensionFilter, "");
return true;
}

Expand Down
17 changes: 7 additions & 10 deletions UI/GameInfoCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,18 @@ std::vector<Path> GameInfo::GetSaveDataDirectories() {
_dbg_assert_(hasFlags & GameInfoFlags::PARAM_SFO); // so we know we have the ID.
Path memc = GetSysDirectory(DIRECTORY_SAVEDATA);

std::vector<File::FileInfo> dirs;
File::GetFilesInDir(memc, &dirs);

std::vector<Path> directories;
if (id.size() < 5) {
// Invalid game ID.
return directories;
}

std::vector<File::FileInfo> dirs;
const std::string &prefix = id;
File::GetFilesInDir(memc, &dirs, nullptr, 0, prefix);

for (size_t i = 0; i < dirs.size(); i++) {
if (startsWith(dirs[i].name, id)) {
directories.push_back(dirs[i].fullName);
}
directories.push_back(dirs[i].fullName);
}

return directories;
Expand Down Expand Up @@ -500,10 +501,6 @@ class GameInfoWorkItem : public Task {
info_->fileType = Identify_File(info_->GetFileLoader().get(), &errorString);
}

if (!info_->Ready(GameInfoFlags::FILE_TYPE) && !(flags_ & GameInfoFlags::FILE_TYPE)) {
_dbg_assert_(false);
}

switch (info_->fileType) {
case IdentifiedFileType::PSP_PBP:
case IdentifiedFileType::PSP_PBP_DIRECTORY:
Expand Down
4 changes: 4 additions & 0 deletions UI/MainScreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,10 @@ std::vector<Path> GameBrowser::GetPinnedPaths() const {
#else
static const std::string sepChars = "/\\";
#endif
if (g_Config.vPinnedPaths.empty()) {
// Early-out.
return std::vector<Path>();
}

const std::string currentPath = File::ResolvePath(path_.GetPath().ToString());
const std::vector<std::string> paths = g_Config.vPinnedPaths;
Expand Down
18 changes: 14 additions & 4 deletions android/src/org/ppsspp/ppsspp/PpssppActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.os.Environment;
import android.os.Looper;
import android.os.ParcelFileDescriptor;
import android.provider.MediaStore;
import android.util.Log;
import android.system.StructStatVfs;
import android.system.Os;
Expand Down Expand Up @@ -161,14 +162,14 @@ private String cursorToString(Cursor c) {
// Filter out any virtual or partial nonsense.
// There's a bunch of potentially-interesting flags here btw,
// to figure out how to set access flags better, etc.
// Like FLAG_SUPPORTS_WRITE etc.
if ((flags & (DocumentsContract.Document.FLAG_PARTIAL | DocumentsContract.Document.FLAG_VIRTUAL_DOCUMENT)) != 0) {
return null;
}

final String mimeType = c.getString(3);
final boolean isDirectory = mimeType.equals(DocumentsContract.Document.MIME_TYPE_DIR);
final String documentName = c.getString(0);
final long size = c.getLong(1);
final long size = isDirectory ? 0 : c.getLong(1);
final long lastModified = c.getLong(4);

String str = "F|";
Expand Down Expand Up @@ -250,15 +251,24 @@ public long computeRecursiveDirectorySize(String uriString) {
// * https://stackoverflow.com/q
// uestions/42186820/documentfile-is-very-slow
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
public String[] listContentUriDir(String uriString) {
public String[] listContentUriDir(String uriString, String prefix) {
Cursor c = null;
try {
Uri uri = Uri.parse(uriString);
final ContentResolver resolver = getContentResolver();
final Uri childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree(
uri, DocumentsContract.getDocumentId(uri));
final ArrayList<String> listing = new ArrayList<>();
c = resolver.query(childrenUri, columns, null, null, null);

String selection = null;
String[] selectionArgs = null;
if (prefix != null) {
selection = MediaStore.Files.FileColumns.DISPLAY_NAME + " LIKE ?";
// Prefix followed by wildcard
selectionArgs = new String[]{ prefix + "%" };
}

c = resolver.query(childrenUri, columns, selection, selectionArgs, null);
if (c == null) {
return new String[]{ "X" };
}
Expand Down

0 comments on commit 398b181

Please sign in to comment.