Skip to content

Batch TreeArtifact metadata through BazelOutputService - #30797

Open
mzhaom wants to merge 1 commit into
bazelbuild:masterfrom
mzhaom:optimize/bazel-output-service-metadata
Open

Batch TreeArtifact metadata through BazelOutputService#30797
mzhaom wants to merge 1 commit into
bazelbuild:masterfrom
mzhaom:optimize/bazel-output-service-metadata

Conversation

@mzhaom

@mzhaom mzhaom commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Description

Keep Skyframe as the central place that discovers and constructs final output
metadata, while allowing ActionOutputMetadataStore to query an
OutputService for all TreeArtifact child statuses in one BatchStat call.

When no batch statter is available, the existing per-child filesystem path is
unchanged. There is no metadata injection from SpawnRunner or
RemoteExecutionService.

Motivation

BazelOutputService already exposes a multi-path BatchStat, but
TreeArtifact construction requested every child's fast digest separately.
For a lazy output filesystem, every request can cross the output-service RPC
boundary even though the service already owns the staged file and directory
metadata.

In a representative remote-output benchmark producing 4,098 files and 528 MiB
of logical output, the profile contained approximately 4,100 one-path
BazelOutputService.BatchStat calls. An equivalent batching patch on Bazel
8.5.1 produced:

Configuration Elapsed time
Eager output download 4.100 s
Lazy output service before batching 1.484 s
Lazy output service with batched metadata 0.896 s
Inaccessible minimal-download roofline 0.549 s

This removes the RPC fanout while deliberately retaining Skyframe's final
filesystem traversal. That distinction is important for actions containing
multiple spawns or a mix of remote and local outputs: metadata is collected
from the final output-service state, rather than copied from an earlier remote
ActionResult.

Build API Changes

No.

Checklist

  • I have added tests for the new use case.
  • Documentation is not applicable because this is an internal remote
    execution optimization without user-facing configuration changes.

Validation

  • bazel test //src/test/java/com/google/devtools/build/lib/skyframe:ActionOutputMetadataStoreTest --test_timeout=60
  • bazel test //src/test/java/com/google/devtools/build/lib/remote:RemoteTests --test_timeout=60
  • bazel build //src:bazel-dev

The regression test stages a nested TreeArtifact, modifies one child before
Skyframe collects metadata, and verifies both the final digest and exactly one
batch request containing all children.

Release Notes

RELNOTES: None

@mzhaom
mzhaom force-pushed the optimize/bazel-output-service-metadata branch from 089c2e9 to 1b573bc Compare August 19, 2026 17:29
@mzhaom
mzhaom marked this pull request as ready for review August 19, 2026 20:24
@mzhaom
mzhaom requested a review from a team as a code owner August 19, 2026 20:24
@github-actions github-actions Bot added team-Performance Issues for Performance teams team-Core Skyframe, bazel query, BEP, options parsing, bazelrc team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Aug 19, 2026
@meisterT
meisterT requested a review from coeuvre August 24, 2026 14:01
@coeuvre

coeuvre commented Aug 24, 2026

Copy link
Copy Markdown
Member

We intentionally moved away from injecting metadata within the SpawnRunner because there are too many edge cases and we wanted skyframe to be the central place to collect metadata. For this optimization, is it possible to "inject" file/dir metadata into BazelOutputService instead?

Keep Skyframe responsible for discovering and constructing final output metadata, but use the OutputService BatchStat implementation to obtain all TreeArtifact child statuses in one request.

This avoids one output-service RPC per child without trusting metadata captured early in SpawnRunner, so later local writes and mixed remote/local tree outputs are still reflected in the final TreeArtifactValue.
@mzhaom
mzhaom force-pushed the optimize/bazel-output-service-metadata branch from 1b573bc to 1fb98c8 Compare August 25, 2026 16:01
@mzhaom mzhaom changed the title Reuse ActionResult metadata with BazelOutputService Batch TreeArtifact metadata through BazelOutputService Aug 25, 2026
@mzhaom

mzhaom commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — yes. I traced this back to the mixed TreeArtifact issue fixed in #16812 and removed the SpawnRunner/RemoteExecutionService metadata injection entirely.

The revised design keeps Skyframe responsible for traversing the final TreeArtifact and constructing its TreeArtifactValue. After traversal, Skyframe sends all child paths in one multi-path BazelOutputService.BatchStat request. The output service is therefore the authoritative source of file/directory status, but Skyframe still decides what metadata belongs to the completed action.

This avoids stale ActionResult metadata when a later/local spawn modifies a mixed tree, while reducing roughly 4,100 one-path RPCs to one request. The new regression test overwrites a staged child before collection and verifies the final digest plus exactly one batch containing all children.

@coeuvre coeuvre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it now looks better. I will import the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR is awaiting review from an assigned reviewer team-Core Skyframe, bazel query, BEP, options parsing, bazelrc team-Performance Issues for Performance teams team-Remote-Exec Issues and PRs for the Execution (Remote) team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants