perf(lock): resolve lockfile-missing packages in parallel

internal/boxcli/featureflag/feature.go

@@ -7,6 +7,7 @@ import (
 	"log/slog"
 	"os"
 	"strconv"
+	"sync"
 	"testing"
 
 	"go.jetify.com/devbox/internal/build"
@@ -43,7 +44,9 @@ func enable(name string) *feature {
 	return features[name]
 }
 
-var logMap = map[string]bool{}
+// logged records which env var overrides have been logged. Enabled can be
+// called from multiple goroutines.
+var logged sync.Map
 
 func (f *feature) Enabled() bool {
 	if f == nil {
@@ -54,9 +57,8 @@ func (f *feature) Enabled() bool {
 		if !on {
 			status = "disabled"
 		}
-		if !logMap[f.name] {
+		if _, loaded := logged.LoadOrStore(f.name, true); !loaded {
 			slog.Debug("feature flag set via environment variable", "feature", f.name, "status", status)
-			logMap[f.name] = true
 		}
 		return on
 	}

internal/boxcli/multi/sync.go

@@ -55,7 +55,7 @@ func SyncLockfiles(pkgs []string) error {
 		}
 
 		if changed {
-			if err = cuecfg.WriteFile(lockfilePath, lockFile); err != nil {
+			if err = cuecfg.WriteFile(lockfilePath, &lockFile); err != nil {
 				return err
 			}
 			fmt.Printf("Updated: %s\n", lockfilePath)

internal/boxcli/multi/sync_test.go

@@ -27,7 +27,7 @@ func TestSyncLockfilesOnlyRequestedPackages(t *testing.T) {
 			f.Packages[key] = &lock.Package{LastModified: lastModified, Resolved: key + "-" + lastModified}
 		}
 		path := filepath.Join(dir, "devbox.lock")
-		if err := cuecfg.WriteFile(path, f); err != nil {
+		if err := cuecfg.WriteFile(path, &f); err != nil {
 			t.Fatal(err)
 		}
 		return path

internal/devpkg/narinfo_cache.go

@@ -54,16 +54,7 @@ func (p *Package) IsInBinaryCache() (bool, error) {
 func FillNarInfoCache(ctx context.Context, packages ...*Package) error {
 	defer debug.FunctionTimer().End()
 
-	eligiblePackages := []*Package{}
-	for _, p := range packages {
-		// IMPORTANT: isEligibleForBinaryCache will call resolve() which is NOT
-		// concurrency safe. Hence, we call it outside of the go-routine.
-		isEligible, err := p.isEligibleForBinaryCache()
-		// If the package is not eligible or there is an error in determining that, then skip it.
-		if isEligible && err == nil {
-			eligiblePackages = append(eligiblePackages, p)
-		}
-	}
+	eligiblePackages := eligibleForBinaryCache(packages)
 	if len(eligiblePackages) == 0 {
 		return nil
 	}
@@ -87,6 +78,37 @@ func FillNarInfoCache(ctx context.Context, packages ...*Package) error {
 	return group.Wait()
 }
 
+// maxConcurrentResolves bounds how many packages are resolved at once. Each
+// resolve can make a search API request or run `nix flake prefetch`.
+const maxConcurrentResolves = 8
+
+// eligibleForBinaryCache returns the packages that are eligible for the binary
+// cache, preserving their order. Checking eligibility resolves packages that
+// aren't in the lockfile yet, so the checks run concurrently.
+func eligibleForBinaryCache(packages []*Package) []*Package {
+	eligible := make([]bool, len(packages))
+	var group errgroup.Group
+	group.SetLimit(maxConcurrentResolves)
+	for i, p := range packages {
+		group.Go(func() error {
+			// If the package is not eligible or there is an error in
+			// determining that, then skip it.
+			isEligible, err := p.isEligibleForBinaryCache()
+			eligible[i] = isEligible && err == nil
+			return nil
+		})
+	}
+	_ = group.Wait()
+
+	var result []*Package
+	for i, p := range packages {
+		if eligible[i] {
+			result = append(result, p)
+		}
+	}
+	return result
+}
+
 // areExpectedOutputsInCacheOnce wraps fetchNarInfoStatusOnce and returns true
 // if the expected outputs are in the cache.
 func (p *Package) areExpectedOutputsInCacheOnce(outputName string) (bool, error) {
@@ -177,8 +199,8 @@ func (p *Package) isEligibleForBinaryCache() (bool, error) {
 // sysInfoIfExists returns the system info for the user's system. If the sysInfo
 // is missing, then nil is returned
 // NOTE: this is called from multiple go-routines and needs to be concurrency safe.
-// Hence, we compute nix.Version, nix.System and lockfile.Resolve prior to calling this
-// function from within a goroutine.
+// nix.Version and nix.System are computed once, and lock.File.Resolve is safe to
+// call concurrently.
 func (p *Package) sysInfoIfExists() (*lock.SystemInfo, error) {
 	if !p.isVersioned() {
 		return nil, nil

internal/devpkg/narinfo_cache_test.go

@@ -8,10 +8,15 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"slices"
+	"strings"
+	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
 
+	"go.jetify.com/devbox/internal/lock"
+	"go.jetify.com/devbox/internal/nix"
 	"go.jetify.com/pkg/filecache"
 )
 
@@ -242,3 +247,70 @@ func TestFetchNarInfoStatusCache(t *testing.T) {
 		}
 	})
 }
+
+// blockingLocker is a lock.Locker whose Resolve blocks until release is
+// closed, and reports each call on inFlight.
+type blockingLocker struct {
+	lockfile
+	inFlight chan struct{}
+	release  chan struct{}
+}
+
+func (l *blockingLocker) Resolve(pkg string) (*lock.Package, error) {
+	l.inFlight <- struct{}{}
+	<-l.release
+
+	entry := &lock.Package{
+		Resolved: "github:NixOS/nixpkgs/" + nixCommitHash + "#" + pkg,
+	}
+	// Packages named "uncached@..." have no system info, so they're not
+	// eligible for the binary cache.
+	if !strings.HasPrefix(pkg, "uncached@") {
+		entry.Systems = map[string]*lock.SystemInfo{nix.System(): {}}
+	}
+	return entry, nil
+}
+
+func TestEligibleForBinaryCacheResolvesConcurrently(t *testing.T) {
+	if !nix.AtLeast(nix.Version2_17) {
+		t.Skip("binary cache eligibility requires nix >= 2.17")
+	}
+
+	raws := []string{"hello@1", "uncached@1", "jq@1", "curl@1"}
+	locker := &blockingLocker{
+		inFlight: make(chan struct{}, len(raws)),
+		release:  make(chan struct{}),
+	}
+	unblock := sync.OnceFunc(func() { close(locker.release) })
+	// Don't leave resolves blocked if the test fails early.
+	t.Cleanup(unblock)
+	pkgs := make([]*Package, len(raws))
+	for i, raw := range raws {
+		pkgs[i] = PackageFromStringWithDefaults(raw, locker)
+	}
+
+	done := make(chan []*Package)
+	go func() { done <- eligibleForBinaryCache(pkgs) }()
+
+	// All resolves must be in flight at once. If eligibility were checked
+	// serially, only the first would start and this would time out.
+	timeout := time.After(10 * time.Second)
+	for range raws {
+		select {
+		case <-locker.inFlight:
+		case <-timeout:
+			t.Fatal("packages were not resolved concurrently")
+		}
+	}
+	unblock()
+
+	got := <-done
+	var gotRaws []string
+	for _, p := range got {
+		gotRaws = append(gotRaws, p.Raw)
+	}
+	want := []string{"hello@1", "jq@1", "curl@1"}
+	if !slices.Equal(gotRaws, want) {
+		t.Errorf("got eligible packages %v, want %v", gotRaws, want)
+	}
+}