diff --git a/Makefile b/Makefile index cc64192b..b831d33d 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build cover test test-integration proto gazelle clean clean-proto run-server run-client-get-graph run-client-changed-targets help +.PHONY: build cover test test-integration bench proto gazelle clean clean-proto run-server run-client-get-graph run-client-changed-targets help # Bazel wrapper BAZEL = ./tools/bazel @@ -21,6 +21,20 @@ test-integration: @$(BAZEL) test //integration:integration_test --test_output=errors --test_env=TANGO_REPO_REMOTE=$$(git rev-parse --show-toplevel) --test_env=HOME=$$HOME @echo "Integration tests passed!" +# Run GetChangedTargets benchmarks against fixed, checked-in commit pairs. +# Measurement only: not part of `make test` / `make test-integration` and not +# run in CI, so a slow benchmark never fails the build. +bench: + @echo "Running benchmarks..." + @$(BAZEL) test //integration:integration_test \ + --test_output=all \ + --test_arg=-test.run=^$$ \ + --test_arg=-test.bench=. \ + --test_arg=-test.benchtime=15x \ + --test_env=TANGO_REPO_REMOTE=$$(git rev-parse --show-toplevel) \ + --test_env=HOME=$$HOME + @echo "Benchmarks complete!" + # Generate protobuf files using protoc proto: @echo "Generating protobuf files with protoc..." @@ -97,6 +111,7 @@ help: @echo " make build - Build all targets" @echo " make test - Run all tests" @echo " make test-integration - Run integration tests (slow)" + @echo " make bench - Run GetChangedTargets benchs (measurement only, not in CI)" @echo " make gazelle - Update BUILD.bazel files" @echo " make clean - Clean generated files and binaries" @echo "" diff --git a/integration/BUILD.bazel b/integration/BUILD.bazel index 40f7eaa2..c71ce849 100644 --- a/integration/BUILD.bazel +++ b/integration/BUILD.bazel @@ -4,7 +4,10 @@ go_test( name = "integration_test", size = "large", timeout = "long", - srcs = ["integration_test.go"], + srcs = [ + "benchmark_test.go", + "integration_test.go", + ], data = glob(["testdata/**"]), tags = [ "integration", @@ -23,6 +26,8 @@ go_test( "@org_uber_go_yarpc//:yarpc", "@org_uber_go_yarpc//api/transport", "@org_uber_go_yarpc//transport/grpc", + "@org_uber_go_zap//:zap", + "@org_uber_go_zap//zapcore", "@org_uber_go_zap//zaptest", ], ) diff --git a/integration/benchmark_test.go b/integration/benchmark_test.go new file mode 100644 index 00000000..7997bdf3 --- /dev/null +++ b/integration/benchmark_test.go @@ -0,0 +1,79 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Benchmarks for GetChangedTargets. Run via `make bench`; not part of +// `make test` / `make test-integration` and never from CI. +package integration_test + +import ( + "testing" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// coldCommitPairs are distinct consecutive commit pairs from repo history. +// Each pair produces a unique treehash, guaranteeing a cache miss per call. +var coldCommitPairs = []struct{ first, second string }{ + {"57162624a45965a7e783072c56561f91c5d4084d", "74d1cd55155e5f4f43aa92b4e0146a0c528a0d96"}, + {"74d1cd55155e5f4f43aa92b4e0146a0c528a0d96", "3d54234a3d4c0d940d651e002c5c79d71f01b120"}, + {"3d54234a3d4c0d940d651e002c5c79d71f01b120", "821c885d304811652fefeeeb2c21e1907bebf7f6"}, + {"821c885d304811652fefeeeb2c21e1907bebf7f6", "ea874d203f37c58b9ba52cde81309d1e28827eaa"}, + {"ea874d203f37c58b9ba52cde81309d1e28827eaa", "c0cd90a8c35e2cc981e32f074f9a55657f44d7dd"}, + {"c0cd90a8c35e2cc981e32f074f9a55657f44d7dd", "8d7e7f93ea68bfb5e37f29df289a54ab8ff79ae1"}, + {"8d7e7f93ea68bfb5e37f29df289a54ab8ff79ae1", "4d41a6f57d2215db7e78c284f7f8f5e13f6ff07d"}, + {"4d41a6f57d2215db7e78c284f7f8f5e13f6ff07d", "b4591e33a40135ed5c2fef8ee4f96db8ab231904"}, + {"b4591e33a40135ed5c2fef8ee4f96db8ab231904", "f2b15a5e058ed0a678f97111080806e26d0239d4"}, + {"f2b15a5e058ed0a678f97111080806e26d0239d4", "fc7289244106a1a21a146729c6f75eb6d17f2649"}, + {"3662f8acd9fffb535e56b670beb0de0d811e4181", "381c6585a765e350326520d4e0a588bc604fa656"}, + {"381c6585a765e350326520d4e0a588bc604fa656", "450d404d4a337ec2809f28c630b126871a88987e"}, + {"450d404d4a337ec2809f28c630b126871a88987e", "3a07829426dc621d37bdbe2038ac85a20bec5ead"}, + {"3a07829426dc621d37bdbe2038ac85a20bec5ead", "7876d3870dd0da4ec94cb49362bb85247b373358"}, + {"7876d3870dd0da4ec94cb49362bb85247b373358", "9cc890fb7a91de8b129a3cc86f362b393655de5b"}, +} + +// BenchmarkGetChangedTargets_Cold measures uncached GetChangedTargets calls. +// Each iteration uses a different commit pair so every call is a cache miss, +// exercising the full pipeline: git checkout, bazel query, compare, stream. +func BenchmarkGetChangedTargets_Cold(b *testing.B) { + remote := repoRemote(b) + logger := zap.New(zapcore.NewNopCore()) + addr := startServerWithLogger(b, remote, logger) + client := newClient(b, addr) + b.ResetTimer() + for i := 0; i < b.N; i++ { + pair := coldCommitPairs[i] + getChangedTargets(b, client, remote, pair.first, pair.second) + } +} + +// BenchmarkGetChangedTargets_Cached measures cached GetChangedTargets calls. +// The cache is primed with a single call before the timed loop, so every +// iteration in the loop is a cache hit measuring streaming overhead only. +func BenchmarkGetChangedTargets_Cached(b *testing.B) { + remote := repoRemote(b) + logger := zap.New(zapcore.NewNopCore()) + addr := startServerWithLogger(b, remote, logger) + client := newClient(b, addr) + + firstSHA := "57162624a45965a7e783072c56561f91c5d4084d" + secondSHA := "74d1cd55155e5f4f43aa92b4e0146a0c528a0d96" + + getChangedTargets(b, client, remote, firstSHA, secondSHA) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + getChangedTargets(b, client, remote, firstSHA, secondSHA) + } +} diff --git a/integration/integration_test.go b/integration/integration_test.go index c3373596..44b83f56 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -40,6 +40,7 @@ import ( "go.uber.org/yarpc" "go.uber.org/yarpc/api/transport" yarpcgrpc "go.uber.org/yarpc/transport/grpc" + "go.uber.org/zap" "go.uber.org/zap/zaptest" ) @@ -48,14 +49,14 @@ const ( configTemplateFile = "testdata/tango-config.yaml.tmpl" ) -func repoRemote(t *testing.T) string { +func repoRemote(t testing.TB) string { t.Helper() remote := os.Getenv("TANGO_REPO_REMOTE") require.NotEmpty(t, remote, "TANGO_REPO_REMOTE must be set (pass --test_env=TANGO_REPO_REMOTE=... to bazel test)") return remote } -func writeConfig(t *testing.T, dir, remote, clonePath, workerPath string) string { +func writeConfig(t testing.TB, dir, remote, clonePath, workerPath string) string { t.Helper() tmpl, err := template.ParseFiles(configTemplateFile) @@ -82,7 +83,12 @@ func writeConfig(t *testing.T, dir, remote, clonePath, workerPath string) string return configPath } -func startServer(t *testing.T, remote string) string { +func startServer(t testing.TB, remote string) string { + t.Helper() + return startServerWithLogger(t, remote, zaptest.NewLogger(t)) +} + +func startServerWithLogger(t testing.TB, remote string, zl *zap.Logger) string { t.Helper() configDir := t.TempDir() @@ -91,7 +97,6 @@ func startServer(t *testing.T, remote string) string { configPath := writeConfig(t, configDir, remote, clonePath, workerPath) - zl := zaptest.NewLogger(t) logger := zl.Sugar() appCtx, cancel := context.WithCancel(context.Background()) @@ -142,7 +147,7 @@ func startServer(t *testing.T, remote string) string { return listener.Addr().String() } -func newClient(t *testing.T, addr string) pb.TangoYARPCClient { +func newClient(t testing.TB, addr string) pb.TangoYARPCClient { t.Helper() grpcTransport := yarpcgrpc.NewTransport() @@ -251,7 +256,7 @@ type parsedChangedTargets struct { Distances map[string]int32 } -func getChangedTargets(t *testing.T, client pb.TangoYARPCClient, remote, firstSHA, secondSHA string) parsedChangedTargets { +func getChangedTargets(t testing.TB, client pb.TangoYARPCClient, remote, firstSHA, secondSHA string) parsedChangedTargets { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), requestTimeout)