Skip to content

perf(orchestrator): store treehash mapping before graph computation - #266

Merged
xytan0056 merged 2 commits into
mainfrom
eager-treehash-cache
Aug 4, 2026
Merged

perf(orchestrator): store treehash mapping before graph computation#266
xytan0056 merged 2 commits into
mainfrom
eager-treehash-cache

Conversation

@xytan0056

Copy link
Copy Markdown
Contributor

Summary

  • Moves the treehash cache write (BuildDescription → treehash) from after graph computation to before it, so concurrent/subsequent requests can resolve the mapping while the current request is still computing the target graph.
  • Makes the write fire-and-forget: failures are logged as warnings but don't fail the request, since the mapping is a performance optimization, not a correctness requirement.
  • The treehash mapping is a few bytes, so the synchronous Put is negligible compared to graph compute — the win is in timing, not concurrency.

Test plan

  • bazel test //orchestrator/... passes
  • make build passes

@xytan0056
xytan0056 requested review from a team as code owners July 31, 2026 17:45
@xytan0056
xytan0056 force-pushed the eager-treehash-cache branch 3 times, most recently from 36ea8f4 to 65ad7aa Compare August 1, 2026 01:20
Comment thread orchestrator/native_orchestrator.go Outdated
if putErr := b.storage.Put(b.appCtx, storage.UploadRequest{
Key: thCachePath,
Reader: bytes.NewReader([]byte(treehash)),
}); putErr != nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't we return the error if the treehash upload fails? the request will fail if there's no commit -> treehash mapping?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

synced offline: Add a metric to track when treehash mapping upload failed

@xytan0056
xytan0056 force-pushed the eager-treehash-cache branch from 65ad7aa to 778476f Compare August 3, 2026 16:42
Move the treehash cache write from after the graph computation to
before it. This lets concurrent or subsequent requests resolve the
BuildDescription→treehash mapping while the current request is still
computing the (potentially slow) target graph.

The write is now fire-and-forget: failures are logged as warnings but
do not fail the request, since the mapping is a performance
optimization, not a correctness requirement.
@xytan0056
xytan0056 force-pushed the eager-treehash-cache branch from 778476f to d083699 Compare August 3, 2026 16:42
@xytan0056
xytan0056 merged commit becf28f into main Aug 4, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants