perf(controller): eliminate redundant treehash storage reads

[Arjunmehta312]

Summary

Avoid rereading treehash values during GetChangedTargets by reading them
once and reusing them across cache lookup, graph lookup, and background
cache population.

Validation

• Treehash storage reads reduced from 6 to 2 in the affected cache-miss
  path.
• Total storage reads reduced from 9 to 5 in the regression scenario.
• Added a regression test covering the redundant-read case.
• make test passes.
• make lint passes.

[CLAassistant]

All committers have signed the CLA.

[xytan0056]

.

[xytan0056]

Thanks for contributing. good catch

[xytan0056]

threading "" as "not provided" through 4 function signatures is fragile — it conflates "caller didn't look it up" with "looked up, doesn't exist in storage."

Consider a separate resolver with memoization instead :

type treehashResolver struct {
    storage storage.Storage
    emitter *metrics.Emitter
    op      string
    mu      sync.Mutex
    cache   map[string]resolvedTreehash
}

type resolvedTreehash struct {
    value string
    err   error
}

func (r *treehashResolver) resolve(ctx context.Context, build entity.BuildDescription) (string, error) {
    key := cachekey.GetTreehashCachePath(build)
    r.mu.Lock()
    if entry, ok := r.cache[key]; ok {
        r.mu.Unlock()
        return entry.value, entry.err
    }
    r.mu.Unlock()
    value, err := readTreehash(ctx, r.storage, build, r.emitter, r.op)
    r.mu.Lock()
    r.cache[key] = resolvedTreehash{value: value, err: err}
    r.mu.Unlock()
    return value, err
}

Create it at the top of GetChangedTargets and let serveChangedTargetsFromCache, getGraph, and cacheComparedTargets call resolver.resolve() directly. Second call for the same build is a map hit , no "" sentinel, no conditional branches, and if a new code path needs the treehash later it just calls resolve() for free.

[Arjunmehta312]

Implemented this approach in the latest commit: replaced the "" sentinel parameters with a request-scoped treehashResolver, which memoizes successful resolutions without caching errors.
Thanks for the suggestion!

[Arjunmehta312]

Rebased onto latest upstream/main (resolved conflict in controller/getchangedtargets.go).

What was done:

• Resolved merge conflict in controller/getchangedtargets.go (upstream added maps import, our branch needed sync - kept both)
• Rebased onto latest upstream/main (includes 10+ new commits from @sbalabanov-zz for BUG fixes)
• All 29 tests pass, race detector passes

Implementation summary:

• Added treehashResolver with memoization (successful values only, no error caching)
• resolveParallel() preserves concurrent reads + sibling cancellation
• Replaced "" sentinel params with request-scoped resolver
• Treehash reads: 6 → 2 (66% reduction), total storage reads: 9 → 5

Test results: All 29 tests pass, race detector passes.

Ready for re-review @xytan0056