FYI @aranguyen: We don't need to modify anything about how paths are rewritten in Java actions to benefit from execution deduplication with this change. We should still look into getting rid of the in-place modification for direct classpath header compilation to avoid the synchronous fallback introduced by this change.

@tjgq I have no idea how to test this short of running a large build in CI. I tested it manually on Bazel itself, with promising results: I no longer get any failures and build times improve noticeably.

Representative build results for bazel build //src/main/java/... with an initially empty disk cache on an M3 MacBook:

# Baseline
INFO: Elapsed time: 102.879s, Critical Path: 24.86s
INFO: 5128 processes: 145 internal, 3076 darwin-sandbox, 1907 worker.

# --experimental_worker_multiplex_sandboxing
INFO: Elapsed time: 115.551s, Critical Path: 29.83s
INFO: 5128 processes: 145 internal, 3076 darwin-sandbox, 1907 worker.

# --experimental_worker_multiplex_sandboxing --experimental_output_paths=strip, with deduplication patched out
INFO: Elapsed time: 95.024s, Critical Path: 27.01s
INFO: 5128 processes: 1438 disk cache hit, 145 internal, 2375 darwin-sandbox, 1170 worker.

# --experimental_worker_multiplex_sandboxing --experimental_output_paths=strip
INFO: Elapsed time: 80.242s, Critical Path: 22.04s
INFO: 5128 processes: 1367 disk cache hit, 145 internal, 2171 darwin-sandbox, 459 deduplicated, 986 worker.

I expect the sandboxing overhead to be lower on Linux, but even with the overhead, path mapping is substantially faster.

I'm not going to insist too much because I know this might be a lot of effort, but: is there some way we can write a test to verify that a fast, async-unsafe spawn waits for a slow, concurrent one to reuse its outputs before proceeding?

I added a Java test that relies on mocking to make waitForAndReuseOutputs controllably slow. I don't think it's guaranteed to catch the issue if it comes back, but I verified that removing the call to awaitAllOutputReuse makes it fail.

The problem is that we can't just make the other spawn slow (it doesn't run anyway), we need to make its output reuse slow. A more realistic test would be possible if we could somehow delay file system operations. Is that something that the test filesystem supports (say, stall updates to /exec/root/bazel-bin/k8-opt/bin/output on a synchronization barrier)?

I'm not going to insist too much because I know this might be a lot of effort, but: is there some way we can write a test to verify that a fast, async-unsafe spawn waits for a slow, concurrent one to reuse its outputs before proceeding?

I added a Java test that relies on mocking to make waitForAndReuseOutputs controllably slow. I don't think it's guaranteed to catch the issue if it comes back, but I verified that removing the call to awaitAllOutputReuse makes it fail.

The problem is that we can't just make the other spawn slow (it doesn't run anyway), we need to make its output reuse slow. A more realistic test would be possible if we could somehow delay file system operations. Is that something that the test filesystem supports (say, stall updates to /exec/root/bazel-bin/k8-opt/bin/output on a synchronization barrier)?

It might be possible with a custom FileSystem implementation; see https://cs.opensource.google/bazel/bazel/+/master:src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java;l=134;drc=3fe227ff418ef81eb5f221625bfd90f52ac70150 for inspiration.

Thanks for the pointer, that looks pretty interesting. Since the main code path here goes through FileSystemUtils.copyFile and not a fixed method on Path, I think that the current approach may even be more robust though (I am looking into changing the implementation to use NIO).

I added an assert that should make it less likely that the test fails open.

@bazel-io fork 7.4.0