[Scummvm-git-logs] scummvm branch-2-7-0-android -> 6e74b4d364f0b924e201e97dd4e045b3621790fd
lephilousophe
noreply at scummvm.org
Sat Mar 4 17:18:52 UTC 2023
This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
17418eb8af ANDROID: Rewrite cache to make it more efficient
86c40287d7 ANDROID: Slightly optimize and rewrite cacheData
6e74b4d364 ANDROID: Don't keep a _cache variable, it's never checked
Commit: 17418eb8afc87f38c961d67effa6dd0723f0df19
https://github.com/scummvm/scummvm/commit/17418eb8afc87f38c961d67effa6dd0723f0df19
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-03-04T18:18:33+01:00
Commit Message:
ANDROID: Rewrite cache to make it more efficient
Instead of caching path string, cache nodes and their children.
This allows a better tracking of what is in cache and what is not.
Use SoftReference to allow for simple cleanup and ensure we have unique
SAFFSNode instances.
Changed paths:
backends/platform/android/org/scummvm/scummvm/SAFFSTree.java
diff --git a/backends/platform/android/org/scummvm/scummvm/SAFFSTree.java b/backends/platform/android/org/scummvm/scummvm/SAFFSTree.java
index 8ea88ce4547..1a8645003b7 100644
--- a/backends/platform/android/org/scummvm/scummvm/SAFFSTree.java
+++ b/backends/platform/android/org/scummvm/scummvm/SAFFSTree.java
@@ -1,12 +1,13 @@
package org.scummvm.scummvm;
import java.io.FileNotFoundException;
+
+import java.lang.ref.SoftReference;
+
+import java.util.ArrayDeque;
import java.util.Arrays;
+import java.util.Collection;
import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.LinkedList;
-import java.util.ListIterator;
-import java.util.Map;
import android.annotation.SuppressLint;
import android.content.ContentResolver;
@@ -32,7 +33,7 @@ public class SAFFSTree {
public static void loadSAFTrees(Context context) {
final ContentResolver resolver = context.getContentResolver();
- _trees = new HashMap<String, SAFFSTree>();
+ _trees = new HashMap<>();
for (UriPermission permission : resolver.getPersistedUriPermissions()) {
final Uri uri = permission.getUri();
if (!DocumentsContract.isTreeUri(uri)) {
@@ -88,14 +89,20 @@ public class SAFFSTree {
public String _documentId;
public int _flags;
+ private HashMap<String, SoftReference<SAFFSNode>> _children;
+ private boolean _dirty;
+
private SAFFSNode() {
}
- private SAFFSNode(SAFFSNode parent, String path, String documentId, int flags) {
+ private SAFFSNode reset(SAFFSNode parent, String path, String documentId, int flags) {
_parent = parent;
_path = path;
_documentId = documentId;
_flags = flags;
+
+ _children = null;
+ return this;
}
private static int computeFlags(String mimeType, int flags) {
@@ -119,40 +126,19 @@ public class SAFFSTree {
}
}
- // Sentinel object
- private static final SAFFSNode NOT_FOUND_NODE = new SAFFSNode();
-
- private static class SAFCache extends LinkedHashMap<String, SAFFSNode> {
- private static final int MAX_ENTRIES = 1000;
-
- public SAFCache() {
- super(16, 0.75f, true);
- }
-
- @Override
- protected boolean removeEldestEntry(Map.Entry<String, SAFFSNode> eldest) {
- return size() > MAX_ENTRIES;
- }
- }
-
private Context _context;
private Uri _treeUri;
private SAFFSNode _root;
private String _treeName;
- private SAFCache _cache;
-
public SAFFSTree(Context context, Uri treeUri) {
_context = context;
_treeUri = treeUri;
- _cache = new SAFCache();
-
- _root = new SAFFSNode(null, "", DocumentsContract.getTreeDocumentId(treeUri), 0);
+ _root = new SAFFSNode().reset(null, "", DocumentsContract.getTreeDocumentId(treeUri), 0);
// Update flags and get name
_treeName = stat(_root);
- clearCache();
}
public String getTreeId() {
@@ -160,98 +146,88 @@ public class SAFFSTree {
}
private void clearCache() {
- _cache.clear();
- _cache.put("/", _root);
- _cache.put("", _root);
- }
-
- private static String[] normalizePath(String path) {
- LinkedList<String> components = new LinkedList<String>(Arrays.asList(path.split("/")));
- ListIterator<String> it = components.listIterator();
- while(it.hasNext()) {
- final String component = it.next();
- if (component.isEmpty()) {
- it.remove();
- continue;
- }
- if (".".equals(component)) {
- it.remove();
+ ArrayDeque<SAFFSNode> stack = new ArrayDeque<>();
+ stack.push(_root);
+ while (stack.size() > 0) {
+ SAFFSNode node = stack.pop();
+ node._dirty = true;
+ if (node._children == null) {
continue;
}
- if ("..".equals(component)) {
- it.remove();
- if (it.hasPrevious()) {
- it.previous();
- it.remove();
+ for (SoftReference<SAFFSNode> ref : node._children.values()) {
+ node = ref.get();
+ if (node != null) {
+ stack.push(node);
}
}
}
- return components.toArray(new String[0]);
}
public SAFFSNode pathToNode(String path) {
- SAFFSNode node = null;
+ String[] components = path.split("/");
- // Short-circuit
- node = _cache.get(path);
- if (node != null) {
- if (node == NOT_FOUND_NODE) {
- return null;
- } else {
- return node;
+ SAFFSNode node = _root;
+ for (String component : components) {
+ if (component.isEmpty() || ".".equals(component)) {
+ continue;
}
- }
-
- String[] components = normalizePath(path);
-
- int pivot = components.length;
- String wpath = "/" + String.join("/", components);
-
- while(pivot > 0) {
- node = _cache.get(wpath);
- if (node != null) {
- break;
+ if ("..".equals(component)) {
+ if (node._parent != null) {
+ node = node._parent;
+ }
+ continue;
}
- // Try without last component
- pivot--;
- int newidx = wpath.length() - components[pivot].length() - 1;
- wpath = wpath.substring(0, newidx);
+ node = getChild(node, component);
+ if (node == null) {
+ return null;
+ }
}
+ return node;
+ }
- // We found a negative result in cache for a point in the path
- if (node == NOT_FOUND_NODE) {
- wpath = "/" + String.join("/", components);
- _cache.put(wpath, NOT_FOUND_NODE);
- _cache.put(path, NOT_FOUND_NODE);
- return null;
+ public SAFFSNode[] getChildren(SAFFSNode node) {
+ Collection<SAFFSNode> results = null;
+ if (node._children != null && !node._dirty) {
+ results = new ArrayDeque<>();
+ for (SoftReference<SAFFSNode> ref : node._children.values()) {
+ if (ref == null) {
+ continue;
+ }
+ SAFFSNode newnode = ref.get();
+ if (newnode == null) {
+ // Some reference went stale: refresh
+ results = null;
+ break;
+ }
+ results.add(newnode);
+ }
}
- // Start from the last cached result (if any)
- if (pivot == 0) {
- node = _root;
- }
- while(pivot < components.length) {
- node = getChild(node, components[pivot]);
- if (node == null) {
- // Cache as much as we can
- wpath = "/" + String.join("/", components);
- _cache.put(wpath, NOT_FOUND_NODE);
- _cache.put(path, NOT_FOUND_NODE);
+ if (results == null) {
+ try {
+ results = fetchChildren(node);
+ } catch (Exception e) {
+ Log.w(ScummVM.LOG_TAG, "Failed to get children: " + e);
return null;
}
-
- pivot++;
}
- _cache.put(path, node);
- return node;
+ return results.toArray(new SAFFSNode[0]);
}
- public SAFFSNode[] getChildren(SAFFSNode node) {
+ public Collection<SAFFSNode> fetchChildren(SAFFSNode node) {
final ContentResolver resolver = _context.getContentResolver();
final Uri searchUri = DocumentsContract.buildChildDocumentsUriUsingTree(_treeUri, node._documentId);
- final LinkedList<SAFFSNode> results = new LinkedList<>();
+ final ArrayDeque<SAFFSNode> results = new ArrayDeque<>();
+
+ // Keep the old children around to reuse them: this will help to keep one SAFFSNode instance for each node
+ HashMap<String, SoftReference<SAFFSNode>> oldChildren = node._children;
+ node._children = null;
+
+ // When _children will be set, it will be clean
+ node._dirty = false;
+ HashMap<String, SoftReference<SAFFSNode>> newChildren = new HashMap<>();
Cursor c = null;
try {
@@ -266,69 +242,82 @@ public class SAFFSTree {
final int ourFlags = SAFFSNode.computeFlags(mimeType, flags);
- SAFFSNode newnode = new SAFFSNode(node, node._path + "/" + displayName, documentId, ourFlags);
- _cache.put(newnode._path, newnode);
+ SAFFSNode newnode = null;
+ SoftReference<SAFFSNode> oldnodeRef = null;
+ if (oldChildren != null) {
+ oldnodeRef = oldChildren.remove(displayName);
+ if (oldnodeRef != null) {
+ newnode = oldnodeRef.get();
+ }
+ }
+ if (newnode == null) {
+ newnode = new SAFFSNode();
+ }
+
+ newnode.reset(node, node._path + "/" + displayName, documentId, ourFlags);
+ newChildren.put(displayName, new SoftReference<>(newnode));
+
results.add(newnode);
}
- } catch (Exception e) {
- Log.w(ScummVM.LOG_TAG, "Failed query: " + e);
+
+ // Success: store the cache
+ node._children = newChildren;
} finally {
if (c != null) {
c.close();
}
}
- return results.toArray(new SAFFSNode[0]);
+ return results;
}
public SAFFSNode getChild(SAFFSNode node, String name) {
final ContentResolver resolver = _context.getContentResolver();
final Uri searchUri = DocumentsContract.buildChildDocumentsUriUsingTree(_treeUri, node._documentId);
- String childPath = node._path + "/" + name;
SAFFSNode newnode;
- newnode = _cache.get(childPath);
- if (newnode != null) {
- if (newnode == NOT_FOUND_NODE) {
+ // This variable is used to hold a strong reference on every children nodes
+ Collection<SAFFSNode> children;
+
+ if (node._children == null || node._dirty) {
+ try {
+ children = fetchChildren(node);
+ } catch (Exception e) {
+ Log.w(ScummVM.LOG_TAG, "Failed to get children: " + e);
return null;
- } else {
- return newnode;
}
}
- Cursor c = null;
- try {
- c = resolver.query(searchUri, new String[] { DocumentsContract.Document.COLUMN_DISPLAY_NAME,
- DocumentsContract.Document.COLUMN_DOCUMENT_ID, DocumentsContract.Document.COLUMN_MIME_TYPE,
- DocumentsContract.Document.COLUMN_FLAGS }, null, null, null);
- while (c.moveToNext()) {
- final String displayName = c.getString(0);
- if (!name.equals(displayName)) {
- continue;
- }
+ SoftReference<SAFFSNode> ref = node._children.get(name);
+ if (ref == null) {
+ return null;
+ }
- final String documentId = c.getString(1);
- final String mimeType = c.getString(2);
+ newnode = ref.get();
+ if (newnode != null) {
+ return newnode;
+ }
- final int flags = c.getInt(3);
+ // Node reference was stale, force a refresh
+ try {
+ children = fetchChildren(node);
+ } catch (Exception e) {
+ Log.w(ScummVM.LOG_TAG, "Failed to get children: " + e);
+ return null;
+ }
- final int ourFlags = SAFFSNode.computeFlags(mimeType, flags);
+ ref = node._children.get(name);
+ if (ref == null) {
+ return null;
+ }
- newnode = new SAFFSNode(node, childPath, documentId, ourFlags);
- _cache.put(newnode._path, newnode);
- return newnode;
- }
- } catch (Exception e) {
- Log.w(ScummVM.LOG_TAG, "Failed query: " + e);
- } finally {
- if (c != null) {
- c.close();
- }
+ newnode = ref.get();
+ if (newnode == null) {
+ Log.e(ScummVM.LOG_TAG, "Failed to keep a reference on object");
}
- _cache.put(childPath, NOT_FOUND_NODE);
- return null;
+ return newnode;
}
public SAFFSNode createDirectory(SAFFSNode node, String name) {
@@ -372,11 +361,9 @@ public class SAFFSTree {
return false;
}
- for(Map.Entry<String, SAFFSNode> e : _cache.entrySet()) {
- if (e.getValue() == node) {
- e.setValue(NOT_FOUND_NODE);
- }
- }
+ // Cleanup node
+ node._parent._dirty = true;
+ node.reset(null, null, null, 0);
return true;
}
@@ -399,6 +386,16 @@ public class SAFFSTree {
final Uri parentUri = DocumentsContract.buildDocumentUriUsingTree(_treeUri, node._documentId);
Uri newDocUri;
+ // Make sure _children is OK
+ if (node._children == null || node._dirty) {
+ try {
+ fetchChildren(node);
+ } catch (Exception e) {
+ Log.w(ScummVM.LOG_TAG, "Failed to get children: " + e);
+ return null;
+ }
+ }
+
try {
newDocUri = DocumentsContract.createDocument(resolver, parentUri, mimeType, name);
} catch(FileNotFoundException e) {
@@ -410,18 +407,29 @@ public class SAFFSTree {
final String documentId = DocumentsContract.getDocumentId(newDocUri);
- final SAFFSNode newnode = new SAFFSNode(node, node._path + "/" + name, documentId, 0);
+ SAFFSNode newnode = null;
+
+ SoftReference<SAFFSNode> oldnodeRef = node._children.remove(name);
+ if (oldnodeRef != null) {
+ newnode = oldnodeRef.get();
+ }
+ if (newnode == null) {
+ newnode = new SAFFSNode();
+ }
+
+ newnode.reset(node, node._path + "/" + name, documentId, 0);
// Update flags
- final String realName = stat(_root);
+ final String realName = stat(newnode);
if (realName == null) {
return null;
}
// Unlikely but...
if (!realName.equals(name)) {
+ node._children.remove(realName);
newnode._path = node._path + "/" + realName;
}
- _cache.put(newnode._path, newnode);
+ node._children.put(realName, new SoftReference<>(newnode));
return newnode;
}
Commit: 86c40287d76744b97bc2fbf5465dcaf3a902e119
https://github.com/scummvm/scummvm/commit/86c40287d76744b97bc2fbf5465dcaf3a902e119
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-03-04T18:18:33+01:00
Commit Message:
ANDROID: Slightly optimize and rewrite cacheData
Changed paths:
backends/fs/android/android-saf-fs.cpp
diff --git a/backends/fs/android/android-saf-fs.cpp b/backends/fs/android/android-saf-fs.cpp
index 5e29dd5b145..65ba1e927f6 100644
--- a/backends/fs/android/android-saf-fs.cpp
+++ b/backends/fs/android/android-saf-fs.cpp
@@ -646,13 +646,12 @@ void AndroidSAFFilesystemNode::cacheData(bool force) {
jstring pathObj = (jstring)env->GetObjectField(_safNode, _FID__path);
const char *path = env->GetStringUTFChars(pathObj, 0);
- if (path != 0) {
- workingPath = Common::String(path);
- env->ReleaseStringUTFChars(pathObj, path);
- } else {
+ if (path == nullptr) {
env->DeleteLocalRef(pathObj);
return;
}
+ workingPath = Common::String(path);
+ env->ReleaseStringUTFChars(pathObj, path);
env->DeleteLocalRef(pathObj);
jstring idObj = (jstring)env->CallObjectMethod(_safTree, _MID_getTreeId);
@@ -672,10 +671,15 @@ void AndroidSAFFilesystemNode::cacheData(bool force) {
}
const char *id = env->GetStringUTFChars(idObj, 0);
- if (id != 0) {
- _path = Common::String::format("%s%s%s", SAF_MOUNT_POINT, id, workingPath.c_str());
- env->ReleaseStringUTFChars(idObj, id);
+ if (id == nullptr) {
+ env->DeleteLocalRef(idObj);
+ return;
}
+
+ _path = Common::String(SAF_MOUNT_POINT);
+ _path += id;
+ _path += workingPath;
+ env->ReleaseStringUTFChars(idObj, id);
env->DeleteLocalRef(idObj);
_cached = true;
Commit: 6e74b4d364f0b924e201e97dd4e045b3621790fd
https://github.com/scummvm/scummvm/commit/6e74b4d364f0b924e201e97dd4e045b3621790fd
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-03-04T18:18:33+01:00
Commit Message:
ANDROID: Don't keep a _cache variable, it's never checked
Changed paths:
backends/fs/android/android-saf-fs.cpp
backends/fs/android/android-saf-fs.h
diff --git a/backends/fs/android/android-saf-fs.cpp b/backends/fs/android/android-saf-fs.cpp
index 65ba1e927f6..ea315d3ed5f 100644
--- a/backends/fs/android/android-saf-fs.cpp
+++ b/backends/fs/android/android-saf-fs.cpp
@@ -249,7 +249,7 @@ AndroidSAFFilesystemNode *AndroidSAFFilesystemNode::makeFromTree(jobject safTree
}
AndroidSAFFilesystemNode::AndroidSAFFilesystemNode(jobject safTree, jobject safNode) :
- _cached(false), _flags(0), _safParent(nullptr) {
+ _flags(0), _safParent(nullptr) {
JNIEnv *env = JNI::getEnv();
@@ -263,7 +263,7 @@ AndroidSAFFilesystemNode::AndroidSAFFilesystemNode(jobject safTree, jobject safN
AndroidSAFFilesystemNode::AndroidSAFFilesystemNode(jobject safTree, jobject safParent,
const Common::String &path, const Common::String &name) :
- _safNode(nullptr), _cached(false), _flags(0), _safParent(nullptr) {
+ _safNode(nullptr), _flags(0), _safParent(nullptr) {
JNIEnv *env = JNI::getEnv();
@@ -296,7 +296,6 @@ AndroidSAFFilesystemNode::AndroidSAFFilesystemNode(const AndroidSAFFilesystemNod
assert(_safParent);
}
- _cached = node._cached;
_path = node._path;
_flags = node._flags;
_newName = node._newName;
@@ -478,7 +477,7 @@ Common::SeekableWriteStream *AndroidSAFFilesystemNode::createWriteStream() {
env->DeleteLocalRef(child);
- cacheData(true);
+ cacheData();
}
jint fd = env->CallIntMethod(_safTree, _MID_createWriteStream, _safNode);
@@ -539,7 +538,7 @@ bool AndroidSAFFilesystemNode::createDirectory() {
env->DeleteLocalRef(child);
- cacheData(true);
+ cacheData();
return true;
}
@@ -612,11 +611,7 @@ void AndroidSAFFilesystemNode::removeTree() {
}
}
-void AndroidSAFFilesystemNode::cacheData(bool force) {
- if (_cached && !force) {
- return;
- }
-
+void AndroidSAFFilesystemNode::cacheData() {
JNIEnv *env = JNI::getEnv();
_flags = env->GetIntField(_safNode, _FID__flags);
@@ -640,7 +635,6 @@ void AndroidSAFFilesystemNode::cacheData(bool force) {
}
env->DeleteLocalRef(nameObj);
}
- _path.clear();
Common::String workingPath;
@@ -648,6 +642,7 @@ void AndroidSAFFilesystemNode::cacheData(bool force) {
const char *path = env->GetStringUTFChars(pathObj, 0);
if (path == nullptr) {
env->DeleteLocalRef(pathObj);
+ error("SAFFSNode::_path is null");
return;
}
workingPath = Common::String(path);
@@ -656,22 +651,23 @@ void AndroidSAFFilesystemNode::cacheData(bool force) {
jstring idObj = (jstring)env->CallObjectMethod(_safTree, _MID_getTreeId);
if (env->ExceptionCheck()) {
- LOGE("SAFFSTree::getTreeId failed");
-
env->ExceptionDescribe();
env->ExceptionClear();
env->ReleaseStringUTFChars(pathObj, path);
env->DeleteLocalRef(pathObj);
+ error("SAFFSTree::getTreeId failed");
return;
}
if (!idObj) {
+ error("SAFFSTree::getTreeId returned null");
return;
}
const char *id = env->GetStringUTFChars(idObj, 0);
if (id == nullptr) {
+ error("Failed to get string from SAFFSTree::getTreeId");
env->DeleteLocalRef(idObj);
return;
}
@@ -681,8 +677,6 @@ void AndroidSAFFilesystemNode::cacheData(bool force) {
_path += workingPath;
env->ReleaseStringUTFChars(idObj, id);
env->DeleteLocalRef(idObj);
-
- _cached = true;
}
const char AddSAFFakeNode::SAF_ADD_FAKE_PATH[] = "/saf";
diff --git a/backends/fs/android/android-saf-fs.h b/backends/fs/android/android-saf-fs.h
index 53e5bbec2bf..f3da7b89d8d 100644
--- a/backends/fs/android/android-saf-fs.h
+++ b/backends/fs/android/android-saf-fs.h
@@ -69,7 +69,6 @@ protected:
// In this case _path is the parent path, _newName the node name and _safParent the parent SAF object
jobject _safNode;
- bool _cached;
Common::String _path;
int _flags;
jobject _safParent;
@@ -156,7 +155,7 @@ protected:
AndroidSAFFilesystemNode(jobject safTree, jobject safParent,
const Common::String &path, const Common::String &name);
- void cacheData(bool force = false);
+ void cacheData();
};
class AddSAFFakeNode final : public AbstractFSNode, public AndroidFSNode {
More information about the Scummvm-git-logs
mailing list