build: run isolated LLVM backends by package

[zhouguangyuan0718]

Stacked on #2181.\n\n- create a worker-local llssa.Program, TargetMachine, and C ABI transformer for each ready package\n- replay package and alternate-runtime compiler directives before worker compilation\n- run ready executable-package DAG levels under the existing -p bound\n- serialize only Plan9 assembly translation, whose dependency currently uses LLVM global context\n- snapshot linker state before disposing each worker Program\n\nValidation:\n- GOCACHE=/private/tmp/llgo-pr8-gocache go test ./internal/build -count=1\n- GOCACHE=/private/tmp/llgo-pr8-race-gocache go test -race ./internal/build -run TestLinkOptionsOmitDWARFPreservesPclntab/w$ -count=1 -v\n- GOCACHE=/private/tmp/llgo-pr8-suite-gocache go test ./cl ./internal/build ./internal/goflags ./cmd/internal/flags -count=1

[fennoai]

Review: build: run isolated LLVM backends by package

Reviewed the changes in this PR's own commit (build: run isolated LLVM backends by package) across internal/build/{build,collect,package_build,plan9asm}.go and the added tests. The isolation design (per-worker backendSession, immutable template, snapshot-before-dispose, serialized Plan9 asm path) is coherent and the level scheduling / cache-hit finalization reuse are handled carefully. The findings below are ordered by severity.

Correctness (concurrency)

• Cache-hit packages still compile on the shared ctx.prog while a level runs concurrently. In buildPreflightedPackageLevel, when canUseIsolatedBackend() && len(level) > 1, every spec in the level runs buildPreflightedPackage(ctx, ...) on the shared coordinator ctx from multiple goroutines. Inside executePackageBuild, the isolated-backend branch is gated on !aPkg.CacheHit, so a cache-hit package falls through to buildPkg(ctx, ...) → preparePackageModule → cl.NewPackageExWithEmbed(ctx.prog, ...) (the early return for cache hits happens after that call). Two or more cache-hit packages in the same level therefore register into the same llssa.Program concurrently — a data race. This is the common incremental-build case (warm cache). See inline note at internal/build/build.go. Worth confirming with go test -race ./internal/build on a build whose ready level contains ≥2 cache-hit packages.

Robustness

• Panics in worker goroutines are not recovered. preparePackageModule (and other frontend paths) use check(err) which panics. On the serial path this unwound to a returned error; on the new pooled path (buildPreflightedPackageLevel, preflightPackageBuilds) a compile error panics inside a worker goroutine with no recover, crashing the process and hanging wg.Wait() instead of being aggregated into firstErr. Consider a defer recover() in each worker that converts the panic into a result{err: ...}.

Performance

• Each worker Program replays the entire program's syntax metadata. newProgram() iterates the full immutable syntaxInputs set (ParsePkgSyntax + PreCollectLinknames over every file of every package) plus preCollectRuntimeLinknames for every per-package session. Since a session is created per compiled package, this is O(packages²) frontend replay and each concurrently-live Program holds its own copy of the program-wide maps (peak memory ≈ parallelism × program-metadata). Consider building this metadata once and sharing it read-only, or pooling one session per worker goroutine rather than one per package. Inline note at internal/build/build.go.
• plan9asmGlobalContextMu is held across module transforms and mod.String(). Only the llvm.GlobalContext-based TranslateSourceModuleForPkg call needs the global lock; holding it across LowerLargeAggregates, TransformModule, and the (potentially expensive) mod.String() serializes that work across all workers for asm-heavy packages (runtime, syscall, internal/*). The sibling plan9asmSigsForPkg already scopes its lock narrowly to just the translate call. Inline note at internal/build/plan9asm.go.

Maintainability

• Unsynchronized lazy nil-init of shared ctx.sfiles / ctx.plan9asm. plan9asmEnabled, getCachedSFiles, and cacheSFiles do if ctx.X == nil { ctx.X = &...{} } as an unguarded write on a shared ctx, reachable from the concurrent worker path. It is currently benign because Do()/initializePackageBuildState always pre-populate these fields, but the dead nil branches imply a safety that isn't there. Recommend removing them and relying on eager init (and consolidating the three sfilesState construction sites, two of which leave cache nil).
• Stale comments. preparePackageModule still says it "remains serial because it updates Program-wide registration state" (internal/build/build.go:1825-1826) — it now runs concurrently against a per-worker Program on the isolated path. compilePackageModule's comment "so later PRs can give it a worker-local backend context" (internal/build/build.go:1873-1875) is now stale since this PR already provides that worker-local context. Please update both.

[fennoai]

Data race on the shared ctx.prog for cache-hit packages. This gate uses the isolated backend only when !aPkg.CacheHit. A cache-hit package falls through to buildPkg(ctx, ...) (the shared ctx passed by buildPreflightedPackageLevel) → preparePackageModule → cl.NewPackageExWithEmbed(ctx.prog, ...), which runs before the cache-hit early return. When canUseIsolatedBackend() && len(level) > 1, workers run concurrently on the shared ctx, so ≥2 cache-hit packages in one level register into the same llssa.Program simultaneously — a race. Route cache-hit frontend registration through an isolated/serialized path too.

[fennoai]

No panic recovery in the worker. buildPreflightedPackage reaches check(err) (which panics) via preparePackageModule/clFile/appendExternalLinkArgs. On the old serial path this surfaced as a returned error; here a panic in a worker goroutine has no recover, so it crashes the process and wg.Wait() never returns instead of being aggregated into firstErr. Add a defer func(){ if r:=recover(); r!=nil { completed <- result{index:index, err: fmt.Errorf("%v", r)} } }() (or return errors instead of panicking on this path).

[fennoai]

O(packages²) frontend replay + duplicated memory. Because a session is created per compiled package, this loop re-walks the full immutable syntaxInputs set (ParsePkgSyntax + PreCollectLinknames over every file of every package), and preCollectRuntimeLinknames runs again below, for every worker Program. Each concurrently-live Program also keeps its own copy of the program-wide linkname/type-background maps, so peak memory scales with parallelism × program-metadata. Consider building this metadata once and sharing it read-only, or pooling one session per worker goroutine instead of one per package.

[fennoai]

Lock scope is wider than the GlobalContext dependency requires. Only TranslateSourceModuleForPkg touches llvm.GlobalContext; holding plan9asmGlobalContextMu across LowerLargeAggregates, TransformModule, and mod.String() (below) serializes those across all workers for asm-heavy packages. plan9asmSigsForPkg already scopes its lock to just the translate call — matching that here would recover most of the parallelism for runtime/stdlib builds.

[fennoai]

Unsynchronized write to a shared ctx field. if ctx.plan9asm == nil { ctx.plan9asm = &plan9asmState{} } (and the equivalent for ctx.sfiles in getCachedSFiles/cacheSFiles) mutates the shared ctx without a lock, and this path is reachable from concurrent workers. It's currently benign only because Do()/initializePackageBuildState always pre-populate these fields — so the nil branch is dead code that implies a safety it doesn't provide. Recommend removing the lazy init and relying on eager initialization.

[codecov]

Codecov Report

❌ Patch coverage is 95.34884% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cl/instr.go	92.59%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!