Skip to content

Commit

Permalink
Use prefix encoding for paths.
Browse files Browse the repository at this point in the history
sandboxfs now has the ability to track directory paths separately from
each mapping request, essentially encoding paths as an integer that
identifies a directory and the file name within that directory.

This change adds support for this kind of encoding to cut down the
massive size of the reconfiguration messages.  As a result, we use less
CPU time in Bazel and in sandboxfs just dealing with communication
overhead.

RELNOTES: None.
PiperOrigin-RevId: 298895132
  • Loading branch information
jmmv authored and katre committed Mar 10, 2020
1 parent 3c7cc74 commit 19e214b
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -160,6 +162,66 @@ public void run() {
}
}

/** A pair that represents the identifier and the path of a prefix as it will be serialized. */
private static class Prefix {
Integer id;
String serializedPath;

/**
* Constructs a new prefix from its components.
*
* @param id the numerical identifier of the prefix
* @param serializedPath the path of the prefix. Given that this is typically comes from a call
* to {@link Path#getParentDirectory()}, the value may be null or empty.
*/
Prefix(Integer id, @Nullable PathFragment serializedPath) {
this.id = id;
if (serializedPath == null || serializedPath.isEmpty()) {
this.serializedPath = "/";
} else {
this.serializedPath = serializedPath.getPathString();
}
}
}

/** Registers and assigns unique identifiers to path prefixes. */
private static class Prefixes {
/** Collection of all known path prefixes to this sandboxfs instance. */
@GuardedBy("this")
private final Map<PathFragment, Integer> prefixes = new HashMap<>();

/** Last identifier assigned to a path prefix. */
@GuardedBy("this")
private Integer lastPrefixId = 1;

/**
* Registers the given path as a prefix.
*
* @param path the path to register as a prefix, which should always be the return value of a
* call to {@link Path#getParentDirectory()}. As a result, this should either be an absolute
* path, or a null or empty path.
* @param newPrefixes tracker for any new prefixes registered by this function. If the given
* path is not yet known, it will be added to this collection.
* @return the identifier for the prefix
*/
private synchronized Integer registerPrefix(PathFragment path, ArrayList<Prefix> newPrefixes) {
Integer id = prefixes.get(path);
if (id != null) {
return id;
}

id = lastPrefixId;
prefixes.put(path, id);
newPrefixes.add(new Prefix(id, path));
lastPrefixId++;

return id;
}
}

/** Tracker for all known path prefixes to this sandboxfs instance. */
private final Prefixes allPrefixes = new Prefixes();

/**
* Initializes a new sandboxfs process instance.
*
Expand Down Expand Up @@ -291,15 +353,28 @@ public void createSandbox(String id, SandboxCreator creator) throws IOException
processStdIn.value(id);
processStdIn.name("m");
processStdIn.beginArray();
ArrayList<Prefix> newPrefixes = new ArrayList<>();
creator.create(
(path, underlyingPath, writable) -> {
Integer pathPrefix =
allPrefixes.registerPrefix(path.getParentDirectory(), newPrefixes);
Integer underlyingPathPrefix =
allPrefixes.registerPrefix(underlyingPath.getParentDirectory(), newPrefixes);

// This synchronized block is theoretically unnecessary because the lock is already
// held by the caller of this lambda. However, we need to reenter it to appease
// the @GuardedBy clauses.
synchronized (this) {
processStdIn.beginObject();
{
processStdIn.name("x");
processStdIn.value(pathPrefix);
processStdIn.name("p");
processStdIn.value(path.getPathString());
processStdIn.value(path.getBaseName());
processStdIn.name("y");
processStdIn.value(underlyingPathPrefix);
processStdIn.name("u");
processStdIn.value(underlyingPath.getPathString());
processStdIn.value(underlyingPath.getBaseName());
if (writable) {
processStdIn.name("w");
processStdIn.value(writable);
Expand All @@ -309,6 +384,15 @@ public void createSandbox(String id, SandboxCreator creator) throws IOException
}
});
processStdIn.endArray();
if (!newPrefixes.isEmpty()) {
processStdIn.name("q");
processStdIn.beginObject();
for (Prefix prefix : newPrefixes) {
processStdIn.name(prefix.id.toString());
processStdIn.value(prefix.serializedPath);
}
processStdIn.endObject();
}
}
processStdIn.endObject();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public void testCreateAndDestroySandboxes() throws IOException {
"{\"id\":\"empty\"}",
"{\"id\":\"sandbox1\"}",
"{\"id\":\"sandbox2\"}",
"{\"id\":\"sandbox1\"}"));
"{\"id\":\"sandbox1\"}",
"{\"id\":\"sandbox3\"}"));

process.createSandbox("sandbox1", (mapper) -> {});
process.createSandbox(
Expand All @@ -60,14 +61,30 @@ public void testCreateAndDestroySandboxes() throws IOException {
mapper.map(PathFragment.create("/a/b/c"), PathFragment.create("/other"), false);
});
process.destroySandbox("sandbox1");
process.createSandbox(
"sandbox3",
(mapper) -> {
// Reuse a previous prefix.
mapper.map(PathFragment.create("/a/b/c"), PathFragment.create("/"), true);
// And create a new prefix to ensure identifiers are not reset.
mapper.map(PathFragment.create("/a/b"), PathFragment.create("/"), false);
});
String expectedRequests =
"{\"C\":{\"i\":\"empty\",\"m\":[]}}"
+ "{\"C\":{\"i\":\"sandbox1\",\"m\":[]}}"
+ "{\"C\":{\"i\":\"sandbox2\",\"m\":["
+ "{\"p\":\"/\",\"u\":\"/some/path\",\"w\":true},"
+ "{\"p\":\"/a/b/c\",\"u\":\"/other\"}"
+ "]}}"
+ "{\"D\":\"sandbox1\"}";
+ "{\"x\":1,\"p\":\"\",\"y\":2,\"u\":\"path\",\"w\":true},"
+ "{\"x\":3,\"p\":\"c\",\"y\":4,\"u\":\"other\"}"
+ "],"
+ "\"q\":{\"1\":\"/\",\"2\":\"/some\",\"3\":\"/a/b\",\"4\":\"/\"}"
+ "}}"
+ "{\"D\":\"sandbox1\"}"
+ "{\"C\":{\"i\":\"sandbox3\",\"m\":["
+ "{\"x\":3,\"p\":\"c\",\"y\":1,\"u\":\"\",\"w\":true},"
+ "{\"x\":5,\"p\":\"b\",\"y\":1,\"u\":\"\"}"
+ "],"
+ "\"q\":{\"5\":\"/a\"}"
+ "}}";

verifyFakeSandboxfsExecution(process, expectedRequests);
}
Expand Down

0 comments on commit 19e214b

Please sign in to comment.