Bazel performs avoidable file-system I/O where in-memory metadata already exists
- Dominant language
- Java
- Stars
- 25.8k
- Forks
- 4.6k
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 72
Description
### Description of the feature request:
While working on #20576 and #30410, I used an agent to audit Bazel for other places that stat, digest, or read files from disk even though the same information is already available in memory. This issue tracks the findings, ranked by expected payoff. Line references are against master at 723c204c28.
Agent output follows:
#### 1. `UploadManifest` re-digests locally executed outputs
When a locally executed action's outputs are uploaded to a remote or disk cache, [`UploadManifest.addFiles`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java#L234-L296) stats and fully re-digests every output. For tree artifacts, [`visitAsFile`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java#L523-L535) calls `statIfFound` and then `digestUtil.compute(path)`, which internally stats *again* instead of taking the status it just obtained. [`RemoteExecutionService.buildUploadManifest`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java#L1744-L1752) adds a third `exists()` call per mandatory output.
The duplicate stats are trivially avoidable by passing the already-obtained `FileStatus` to `digestUtil.compute`. Avoiding the digest read itself is the standing TODO in `addFiles`: `SpawnResult` carries no per-output `FileArtifactValue` and the `OutputMetadataStore` has not been populated yet when `uploadOutputs` runs from `AbstractSpawnStrategy`, so the spawn runner would need to record output metadata into the `SpawnResult`. This is the biggest remaining win for the common local-execution + remote-cache CI configuration; the digest read is currently deduplicated against later consumers only by the global `--cache_computed_file_digests` cache, which is capped at 50k entries.
#### 2. Redundant stat for every manually digested file
[`ActionOutputMetadataStore.constructFileArtifactValue`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java#L453) calls `DigestUtils.manuallyComputeDigest(path)` while holding the file's fresh stat in `statAndValue.statNoFollow()`. Since the digest cache is enabled by default, [`manuallyComputeDigest`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java#L171-L172) immediately re-stats the same path just to build its cache key. The `manuallyComputeDigest(Path, FileStatus)` overload already exists; the stat can be passed through in the non-symlink branch. The same pattern exists in [`FileArtifactValue.createFromStat`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java#L356), which drops the `stat` it was handed on the way to `getDigestWithManualFallback`.
This costs one wasted stat syscall per output on every local execution and action-cache-verification path — tens of thousands per large build on filesystems without a fast xattr digest.
#### 3. Execution log writers ignore tree artifact metadata
The compact and expanded spawn logs digest [outputs with `inputMetadataProvider= null`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java#L261-L288) (an `isFile()` stat, then another stat plus a full digest inside `computeDigest`), and re-`readdir` and re-digest input tree artifacts from disk ([`expandDirectory`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java#L627-L657), [`listDirectoryContents`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java#L359-L393)) even though `InputMetadataProvider.getTreeMetadata` already holds every child's digest. The input-tree side has no ordering problem and could reuse the tree metadata directly; the output side is blocked by the same `SpawnResult`/`OutputMetadataStore` ordering as finding 1. Regular file inputs already reuse metadata correctly.
#### 4. `SingleBuildFileCache` bypasses the syscall cache
[`SingleBuildFileCache.getInputMetadataChecked`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java#L92-L97) issues `path.stat(Symlinks.FOLLOW)` directly, although the `XattrProvider` it holds *is* the command's `SyscallCache` (see the TODO on line 95). Skyframe stats the same source files through the syscall cache when building their `FileStateValue`, so the stat is duplicated for every source input digested through this cache. Routing the stat through `SyscallCache.statIfFound` deduplicates it.
#### 5. Coverage report logs are re-read for BES upload
`coverage_report.lcov` and `baseline_report.lcov` are genuine action outputs with digests in Skyframe, but [`BazelCoverageReportModule`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java#L150-L151) registers them via `BuildToolLogCollection.addLocalFile(name, path)`, which creates a digest-less `LocalFile`. Since they are `LOG`-typed, the BES uploader fully re-reads both files in every `--remote_build_event_upload` mode. Fixing this requires threading the coverage action's `FileArtifactValue` into `BuildToolLogCollection` (a metadata-carrying `addLocalFile` overload already exists).
#### 6. Lower-priority tail
* [`RunfilesTreeUpdater`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java#L126-L127) digests the *input* manifest — an action output with known metadata — from disk on every runfiles staging under `--nobuild_runfile_links`. (The output manifest read is legitimate: it reflects possibly-stale on-disk state.)
* [`ActionOutputMetadataStore.setPathPermissionsIfFile`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java#L593-L600) stats each output before `constructFileArtifactValue` stats it again; the first stat is wasted whenever no chmod is needed.
* [`FilesystemValueChecker`](https://github.com/bazelbuild/bazel/blob/723c204c28/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java#L538-L543) issues a fresh `getLastModifiedTime` stat for outputs it just determined to be dirty; the batch-stat path already reuses the stat it has.
#### Verified as already optimal (to save others the re-audit)
`WorkerFilesHash`/worker key computation, `MerkleTreeComputer`, `ExecutionGraphModule`, the in-memory `.d`-file path in `CppCompileAction`, include scanning's memoized existence cache, `LocalFilesArtifactUploader` (no I/O at all), the BEP transports, and the `ActionCacheChecker`↔`checkOutputs` interplay (memoized; one stat+digest per output per build) all reuse in-memory metadata correctly.
#### Not fixable by metadata reuse
Action stdout/stderr (`FileOutErr`) and Bazel-written logs (JSON profile, execution log, memory dumps) are never digested anywhere, so their upload-time read cannot be avoided by reuse — that would require computing a streaming digest while writing, which is a separate feature.
### Which category does this issue belong to?
Performance, Remote Execution
### What underlying problem are you trying to solve with this feature?
Reduce redundant stat/digest/read syscalls on warm and cached builds, following up on #20576 and #30410.
Contributor guide
Assessment
This issue has not been assessed yet.