Skip to content

Commit 70e3e02

Browse files
authored
[perf] Improve install and ensure perf (#2076)
## Summary This improves install and ensure perf by making the following changes: * Batch all store path existence checks. (previously we did one by one, each `nix ls` call is about 50ms) * If lockfile has store paths we use them (instead of trying to get them from nix) * Fix regression in `FillNarInfoCache` ## How was it tested? Installed large devbox.json (30+ packages) and used `DEVBOX_PRINT_EXEC_TIME` and observed almost all "prep" time disappear. prep time is the work devbox does before actually building the packages.
1 parent 3f555f9 commit 70e3e02

File tree

5 files changed

+101
-33
lines changed

5 files changed

+101
-33
lines changed

internal/devbox/packages.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,7 @@ func (d *Devbox) packagesToInstallInStore(ctx context.Context, mode installMode)
539539

540540
// Second, check which packages are not in the nix store
541541
packagesToInstall := []*devpkg.Package{}
542+
storePathsForPackage := map[*devpkg.Package][]string{}
542543
for _, pkg := range packages {
543544
installables, err := pkg.Installables()
544545
if err != nil {
@@ -549,16 +550,35 @@ func (d *Devbox) packagesToInstallInStore(ctx context.Context, mode installMode)
549550
packagesToInstall = append(packagesToInstall, pkg)
550551
continue
551552
}
552-
storePaths, err := nix.StorePathsFromInstallable(ctx, installable, pkg.HasAllowInsecure())
553+
554+
resolvedStorePaths, err := pkg.GetResolvedStorePaths()
553555
if err != nil {
554-
return nil, packageInstallErrorHandler(err, pkg, installable)
556+
return nil, err
557+
}
558+
if resolvedStorePaths != nil {
559+
storePathsForPackage[pkg] = append(storePathsForPackage[pkg], resolvedStorePaths...)
560+
continue
555561
}
556-
isInStore, err := nix.StorePathsAreInStore(ctx, storePaths)
562+
563+
storePathsForPackage[pkg], err = nix.StorePathsFromInstallable(
564+
ctx, installable, pkg.HasAllowInsecure())
557565
if err != nil {
558-
return nil, err
566+
return nil, packageInstallErrorHandler(err, pkg, installable)
559567
}
560-
if !isInStore {
568+
}
569+
}
570+
571+
// Batch this for perf
572+
storePathMap, err := nix.StorePathsAreInStore(ctx, lo.Flatten(lo.Values(storePathsForPackage)))
573+
if err != nil {
574+
return nil, err
575+
}
576+
577+
for pkg, storePaths := range storePathsForPackage {
578+
for _, storePath := range storePaths {
579+
if !storePathMap[storePath] {
561580
packagesToInstall = append(packagesToInstall, pkg)
581+
break
562582
}
563583
}
564584
}

internal/devpkg/narinfo_cache.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ func (p *Package) areExpectedOutputsInCacheOnce(outputName string) (bool, error)
132132
func (p *Package) fetchNarInfoStatusOnce(
133133
outputName string,
134134
) (map[string]string, error) {
135-
defer debug.FunctionTimer().End()
136135
ctx := context.TODO()
137136

138137
outputToCache := map[string]string{}

internal/devpkg/package.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,3 +715,22 @@ func (p *Package) GetOutputsWithCache() ([]Output, error) {
715715
}
716716
return outputs, nil
717717
}
718+
719+
// GetResolvedStorePaths returns the store paths that are resolved (in lockfile)
720+
func (p *Package) GetResolvedStorePaths() ([]string, error) {
721+
names, err := p.outputs.GetNames(p)
722+
if err != nil {
723+
return nil, err
724+
}
725+
storePaths := []string{}
726+
for _, name := range names {
727+
outputs, err := p.outputsForOutputName(name)
728+
if err != nil {
729+
return nil, err
730+
}
731+
for _, output := range outputs {
732+
storePaths = append(storePaths, output.Path)
733+
}
734+
}
735+
return storePaths, nil
736+
}

internal/nix/store.go

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,49 +48,77 @@ func StorePathsFromInstallable(ctx context.Context, installable string, allowIns
4848

4949
return nil, err
5050
}
51-
return parseStorePathFromInstallableOutput(installable, resultBytes)
51+
52+
validPaths, err := parseStorePathFromInstallableOutput(resultBytes)
53+
if err != nil {
54+
return nil, fmt.Errorf("failed to parse path-info for %s: %w", installable, err)
55+
}
56+
57+
return maps.Keys(validPaths), nil
5258
}
5359

54-
// StorePathsAreInStore returns true if the store path is in the store
55-
// It relies on `nix store ls` to check if the store path is in the store
56-
func StorePathsAreInStore(ctx context.Context, storePaths []string) (bool, error) {
60+
// StorePathsAreInStore a map of store paths to whether they are in the store.
61+
func StorePathsAreInStore(ctx context.Context, storePaths []string) (map[string]bool, error) {
5762
defer debug.FunctionTimer().End()
63+
if len(storePaths) == 0 {
64+
return map[string]bool{}, nil
65+
}
66+
args := append([]string{"path-info", "--offline", "--json"}, storePaths...)
67+
cmd := commandContext(ctx, args...)
68+
debug.Log("Running cmd %s", cmd)
69+
output, err := cmd.Output()
70+
if err != nil {
71+
return nil, err
72+
}
73+
74+
validPaths, err := parseStorePathFromInstallableOutput(output)
75+
if err != nil {
76+
return nil, err
77+
}
78+
79+
result := map[string]bool{}
5880
for _, storePath := range storePaths {
59-
cmd := commandContext(ctx, "store", "ls", storePath)
60-
debug.Log("Running cmd %s", cmd)
61-
if err := cmd.Run(); err != nil {
62-
if exitErr := (&exec.ExitError{}); errors.As(err, &exitErr) {
63-
return false, nil
64-
}
65-
return false, err
66-
}
81+
_, ok := validPaths[storePath]
82+
result[storePath] = ok
6783
}
68-
return true, nil
84+
85+
return result, nil
86+
}
87+
88+
// Older nix versions (like 2.17) are an array of objects that contain path and valid fields
89+
type pathInfoLegacy struct {
90+
Path string `json:"path"`
91+
Valid bool `json:"valid"`
6992
}
7093

7194
// parseStorePathFromInstallableOutput parses the output of `nix store path-from-installable --json`
7295
// This function is decomposed out of StorePathFromInstallable to make it testable.
73-
func parseStorePathFromInstallableOutput(installable string, output []byte) ([]string, error) {
74-
// Newer nix versions (like 2.20)
96+
func parseStorePathFromInstallableOutput(output []byte) (map[string]any, error) {
97+
// Newer nix versions (like 2.20) have output of the form
98+
// {"<store-path>": {}}
99+
// if a store path is used as an installable, the keys will be present even if invalid but
100+
// the values will be null.
75101
var out1 map[string]any
76102
if err := json.Unmarshal(output, &out1); err == nil {
77-
return maps.Keys(out1), nil
103+
maps.DeleteFunc(out1, func(k string, v any) bool {
104+
return v == nil
105+
})
106+
return out1, nil
78107
}
79108

80-
// Older nix versions (like 2.17)
81-
var out2 []struct {
82-
Path string `json:"path"`
83-
Valid bool `json:"valid"`
84-
}
109+
var out2 []pathInfoLegacy
110+
85111
if err := json.Unmarshal(output, &out2); err == nil {
86-
res := []string{}
112+
res := map[string]any{}
87113
for _, outValue := range out2 {
88-
res = append(res, outValue.Path)
114+
if outValue.Valid {
115+
res[outValue.Path] = true
116+
}
89117
}
90118
return res, nil
91119
}
92120

93-
return nil, fmt.Errorf("failed to parse store path from installable (%s) output: %s", installable, output)
121+
return nil, fmt.Errorf("failed to parse path-info output: %s", output)
94122
}
95123

96124
// DaemonError reports an unsuccessful attempt to connect to the Nix daemon.

internal/nix/store_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package nix
33
import (
44
"slices"
55
"testing"
6+
7+
"golang.org/x/exp/maps"
68
)
79

810
func TestParseStorePathFromInstallableOutput(t *testing.T) {
@@ -19,18 +21,18 @@ func TestParseStorePathFromInstallableOutput(t *testing.T) {
1921
},
2022
{
2123
name: "go-basic-nix-2-17-0",
22-
input: `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0","valid":false}]`,
24+
input: `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0","valid":true}]`,
2325
expected: []string{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"},
2426
},
2527
}
2628

2729
for _, tc := range testCases {
2830
t.Run(tc.name, func(t *testing.T) {
29-
actual, err := parseStorePathFromInstallableOutput(tc.name, []byte(tc.input))
31+
actual, err := parseStorePathFromInstallableOutput([]byte(tc.input))
3032
if err != nil {
3133
t.Errorf("Expected no error but got error: %s", err)
3234
}
33-
if !slices.Equal(tc.expected, actual) {
35+
if !slices.Equal(tc.expected, maps.Keys(actual)) {
3436
t.Errorf("Expected store path %s but got %s", tc.expected, actual)
3537
}
3638
})

0 commit comments

Comments
 (0)