Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions ci/llgo-size/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,17 @@ and the Go+ toolchain. The GORM schema package is a real MethodByName consumer:
package-global readonly `callbackTypes` slice supplies the callback names used
by `reflect.Value.MethodByName`. It therefore exercises the plugin through the
same `go test -c` path as the other test-mode results.
The etcdctl case sets `BuildMode = "build"` and exercises the new `go build` /
`llgo build` path against `go.etcd.io/etcd/etcdctl/v3`. All configurations use
Bent's configured build concurrency.
The etcdctl, XGo, and iXGo suites set `BuildMode = "build"` and exercise the
`go build` / `llgo build` path. The other six cases use Bent's default
`BuildMode = "test"` and measure test binaries produced by `go test -c` or
`llgo test -c`.

The deadcode-drop configuration starts every test-mode binary once with empty
test and benchmark selections before its size is accepted. This executes the
generated test-main initialization without running a workload, catching invalid
method pruning such as an `unreachable method called` failure. Ordinary main
binaries are not executed by this check. All configurations use Bent's
configured build concurrency.

Every benchmark/configuration pair is built exactly once. LLGo's package cache
separates archives that contain LTO plugin markers from ordinary archives, so
Expand Down
7 changes: 7 additions & 0 deletions cmd/bent/bent.go
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,13 @@ results will also appear in 'bench'.
}

if buildOnly {
if len(getAndBuildFailures) > 0 {
fmt.Fprintln(os.Stderr, "Get and build failures:")
for _, failure := range getAndBuildFailures {
fmt.Fprintln(os.Stderr, failure)
}
os.Exit(1)
}
return
}

Expand Down
68 changes: 68 additions & 0 deletions cmd/bent/bent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"path"
"reflect"
"runtime"
"strings"
"testing"
)

Expand Down Expand Up @@ -168,6 +169,73 @@ func TestCompileOneBuildsMainPackage(t *testing.T) {
}
}

func TestCompileOneValidatesTestBinary(t *testing.T) {
goCommand, err := exec.LookPath("go")
if err != nil {
t.Fatal(err)
}

tests := []struct {
name string
testSource string
wantFailure bool
}{
{
name: "starts",
testSource: "package sample\n\nimport \"testing\"\n\nfunc TestSmoke(t *testing.T) {}\n",
},
{
name: "startup failure",
testSource: "package sample\n\nimport (\"os\"; \"testing\")\n\nfunc TestMain(*testing.M) { os.Exit(23) }\n",
wantFailure: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
workspace := t.TempDir()
buildDir := t.TempDir()
if err := os.MkdirAll(path.Join(workspace, "testbin"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path.Join(buildDir, "go.mod"), []byte("module example.com/sample\n\ngo 1.22\n"), 0o644); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path.Join(buildDir, "sample_test.go"), []byte(tt.testSource), 0o644); err != nil {
t.Fatal(err)
}

oldDirs, oldDefaultEnv, oldReportBuildTime := dirs, defaultEnv, reportBuildTime
defer func() {
dirs = oldDirs
defaultEnv = oldDefaultEnv
reportBuildTime = oldReportBuildTime
}()
dirs = &directories{wd: workspace, testBinDir: "testbin"}
defaultEnv = replaceEnv(os.Environ(), "GOCACHE", t.TempDir())
reportBuildTime = false

config := Configuration{Name: "Validated", Compiler: goCommand, UseBuildCache: true, ValidateTestBinary: true}
benchmark := Benchmark{Name: "sample", Suite: "sample", Repo: ".", buildDir: buildDir, NotSandboxed: true}
failure := config.compileOne(&benchmark, workspace, 1, false)
if tt.wantFailure {
if !strings.Contains(failure, "startup failed") {
t.Fatalf("compileOne failure = %q, want startup failure", failure)
}
} else if failure != "" {
t.Fatalf("compileOne returned unexpected failure: %s", failure)
}
})
}
}

func TestValidateTestBinarySkipsBuildMode(t *testing.T) {
config := Configuration{ValidateTestBinary: true}
benchmark := Benchmark{BuildMode: buildModeBuild}
if err := config.validateTestBinary(&benchmark, "/does/not/exist", nil); err != nil {
t.Fatalf("validateTestBinary(build mode) = %v, want nil", err)
}
}

// bentCmd returns a "bent" command (that is implemented by rerunning the current program after setting
// BENT_TEST_IS_CMD_BENT). The command is always run in the temporary directory created by TestMain.
func bentCmd(t *testing.T, args ...string) *exec.Cmd {
Expand Down
3 changes: 3 additions & 0 deletions cmd/bent/configs/benchmarks-llgo-size.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
# Kubernetes, logging, TOML, GORM, and etcdctl. They are
# deliberately limited to keep the six-way compiler matrix practical on a
# GitHub hosted runner, and have been verified against the pinned LLGo revision.
# Entries use Bent's default BuildMode = "test" unless their suite explicitly
# selects BuildMode = "build". The latter currently applies to etcdctl, XGo,
# and iXGo.

[[Benchmarks]]
Name = "toml"
Expand Down
3 changes: 3 additions & 0 deletions cmd/bent/configs/configurations-llgo-size.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
Root = "$GOROOT"
OmitVetFlag = true
UseBuildCache = true
# A smaller result is useful only if the method-pruned test binary can start.
# Empty selections exercise test-main initialization without running tests.
ValidateTestBinary = true
BuildFlags = ["-deadcodedrop"]
AfterBuild = ["benchsize"]

Expand Down
76 changes: 58 additions & 18 deletions cmd/bent/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package main
import (
"bufio"
"bytes"
"context"
"fmt"
"io"
"os"
Expand All @@ -25,24 +26,25 @@ import (
// initiate a bent run. These structures are read from a .toml file at
// boot-time.
type Configuration struct {
Name string // Short name used for binary names, mention on command line
Root string // Specific Go root to use for this trial
Compiler string // Optional go-compatible compiler command; defaults to Go from Root
OmitVetFlag bool // Do not pass Go's -vet=off flag to this compiler
UseBuildCache bool // Reuse package-cache entries unless Bent's -a flag is explicitly requested
PgoGen string // Name of sub-directory to put profiles for later loading
PgoUse string // Name of sub-directory to take generated profile files
BuildFlags []string // BuildFlags supplied to the configured build command (e.g., "-p 1")
AfterBuild []string // Array of commands to run, output of all commands for a configuration (across binaries) is collected in <runstamp>.<config>.<cmd>
GcFlags string // GcFlags supplied to the configured build command
LdFlags string // LdFlags supplied to the configured build command
GcEnv []string // Environment variables supplied to the configured build command
RunFlags []string // Extra flags passed to every runnable test binary
RunEnv []string // Extra environment variables passed to the runnable test binary
RunWrapper []string // (Outermost) Command and args to precede the runnable test binary; may fail in the sandbox.
Disabled bool // True if this configuration is temporarily disabled
benchWriter *os.File
rootCopy string // The contents of GOROOT are copied here to isolate compilation benchmarking.
Name string // Short name used for binary names, mention on command line
Root string // Specific Go root to use for this trial
Compiler string // Optional go-compatible compiler command; defaults to Go from Root
OmitVetFlag bool // Do not pass Go's -vet=off flag to this compiler
UseBuildCache bool // Reuse package-cache entries unless Bent's -a flag is explicitly requested
PgoGen string // Name of sub-directory to put profiles for later loading
PgoUse string // Name of sub-directory to take generated profile files
BuildFlags []string // BuildFlags supplied to the configured build command (e.g., "-p 1")
AfterBuild []string // Array of commands to run, output of all commands for a configuration (across binaries) is collected in <runstamp>.<config>.<cmd>
ValidateTestBinary bool // Start test binaries without selecting tests after building; ignored for BuildMode="build"
GcFlags string // GcFlags supplied to the configured build command
LdFlags string // LdFlags supplied to the configured build command
GcEnv []string // Environment variables supplied to the configured build command
RunFlags []string // Extra flags passed to every runnable test binary
RunEnv []string // Extra environment variables passed to the runnable test binary
RunWrapper []string // (Outermost) Command and args to precede the runnable test binary; may fail in the sandbox.
Disabled bool // True if this configuration is temporarily disabled
benchWriter *os.File
rootCopy string // The contents of GOROOT are copied here to isolate compilation benchmarking.
}

var dirs *directories // constant across all configurations, useful in other contexts.
Expand Down Expand Up @@ -254,6 +256,12 @@ func (config *Configuration) compileOne(bench *Benchmark, cwd string, count int,
bench.Disabled = true // if it won't compile, it won't run, either.
return s + "(" + bench.Name + ")\n"
}
if err := config.validateTestBinary(bench, compileTo, cmdEnv); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P3] Validation re-runs on every build repeat, not once per target

compileOne (and therefore validateTestBinary) is invoked buildCount times per benchmark/config pair. With -a N / randomized repeats, the same target is compiled to distinctly-named binaries and re-validated on every repeat. Since a pruned binary's startup behavior doesn't change between identical builds, validation is redundant across repeats. Consider gating validation on count == 0, mirroring how runOtherBenchmarks is already gated at configuration.go:317.

(Skip if per-repeat re-validation is intentional, e.g. to catch nondeterministic build flakiness.)

s := fmt.Sprintf("There was an error validating the test binary for %s (%s): %v", bench.Name, config.Name, err)
fmt.Println(s + "\nDISABLING benchmark " + bench.Name)
bench.Disabled = true
return s + "\n"
}

if reportBuildTime {
// Report and record build stats to testbin
Expand Down Expand Up @@ -313,6 +321,38 @@ func (config *Configuration) compileOne(bench *Benchmark, cwd string, count int,
return ""
}

func (config *Configuration) validateTestBinary(bench *Benchmark, binary string, env []string) error {
if !config.ValidateTestBinary || !bench.buildsTestBinary() {
return nil
}

targetGOOS := getenv(env, "GOOS")
if targetGOOS != "" && targetGOOS != runtime.GOOS {
return fmt.Errorf("cannot run %s binary on %s", targetGOOS, runtime.GOOS)
}
targetGOARCH := getenv(env, "GOARCH")
if targetGOARCH != "" && targetGOARCH != runtime.GOARCH {
return fmt.Errorf("cannot run %s binary on %s", targetGOARCH, runtime.GOARCH)
}

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
cmd := exec.CommandContext(ctx, binary, "-test.run=^$", "-test.bench=^$")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Validation bypasses the Docker sandbox for sandboxed benchmarks

For benchmarks with NotSandboxed == false (the default), the normal run path executes the built test binary inside docker run --net=none ... (bent.go:1277) precisely because these are third-party benchmark binaries fetched and built by the harness.

validateTestBinary executes that same freshly-built binary directly on the host with no container and no network isolation. The GOOS/GOARCH guard above only blocks cross-arch execution; on a Linux host it passes, so the binary's package init(), global initializers, and any TestMain run on the host with full network/filesystem access, even with -test.run=^$ -test.bench=^$.

Consider skipping validation when a sandbox would otherwise be used (i.e. only validate NotSandboxed benchmarks), or running validation through the same Docker sandbox. At minimum, document this trade-off on the ValidateTestBinary field.

cmd.Dir = bench.BuildDir()
cmd.Env = env
if verbose > 0 {
fmt.Println(asCommandLine(dirs.wd, cmd))
}
output, err := cmd.CombinedOutput()
if ctx.Err() == context.DeadlineExceeded {
return fmt.Errorf("startup timed out after 30s")
}
if err != nil {
return fmt.Errorf("startup failed: %w, output = %s", err, output)
}
return nil
}

// say writes s to c's benchmark output file
func (c *Configuration) say(s string) {
b := []byte(s)
Expand Down
Loading