Skip to content

Commit 712ba55

Browse files
authored
[perf] Improve how we determine attribute path (#1050)
## Summary This makes 2 improvements to our use of attribute paths which was causing performance issues: **Improve ProfileListIndex:** ProfileListIndex will attempt to find an exact match in unlocked profile references. (See `normalizedDevboxPackageReference`. This short circuits having to loop through all profile list items and having to compute normalized attribute paths for each. **Split PackageAttributePath:** Split `PackageAttributePath` into `NormalizedPackageAttributePath` and a faster new `PackageAttributePath`. When computing attribute paths we would always use nix search to ensure the attribute path is normalized. This is helpful for comparing 2 distinct attribute paths that point to same package (e.g. `flake` and `flake#default` are likely the same). The downside is that search is slow. (about 100ms per search). So what we do is when using devbox packages, we assume they resolve to nixpkgs and therefore we can reconstruct the exact attribute path (`legacyPackages.<system>.<pkg>`) without using search. Some additional improvements: * Currently local/remote flakes still need to use search. This affects our plugins. We could cache the search results to make them faster. ## How was it tested? Created devbox.json with 20 packages and stuff got super slow. After these changes, it reduces non-installation time during `devbox add` from 40-60 seconds to about 2s.
1 parent 89804f9 commit 712ba55

File tree

3 files changed

+80
-28
lines changed

3 files changed

+80
-28
lines changed

internal/impl/packages.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -277,15 +277,19 @@ func (d *Devbox) removePackagesFromProfile(ctx context.Context, pkgs []string) e
277277
return err
278278
}
279279

280-
for _, pkg := range pkgs {
280+
for _, input := range nix.InputsFromStrings(pkgs, d.lockfile) {
281281
index, err := nix.ProfileListIndex(&nix.ProfileListIndexArgs{
282282
Lockfile: d.lockfile,
283283
Writer: d.writer,
284-
Pkg: pkg,
284+
Input: input,
285285
ProfileDir: profileDir,
286286
})
287287
if err != nil {
288-
ux.Ferror(d.writer, "Package %s not found in profile. Skipping.\n", pkg)
288+
ux.Ferror(
289+
d.writer,
290+
"Package %s not found in profile. Skipping.\n",
291+
input.Raw,
292+
)
289293
continue
290294
}
291295

@@ -322,16 +326,16 @@ func (d *Devbox) pendingPackagesForInstallation(ctx context.Context) ([]string,
322326
if err != nil {
323327
return nil, err
324328
}
325-
for _, pkg := range d.Packages() {
329+
for _, input := range d.packagesAsInputs() {
326330
_, err := nix.ProfileListIndex(&nix.ProfileListIndexArgs{
327331
List: list,
328332
Lockfile: d.lockfile,
329333
Writer: d.writer,
330-
Pkg: pkg,
334+
Input: input,
331335
ProfileDir: profileDir,
332336
})
333337
if err != nil {
334-
pending = append(pending, pkg)
338+
pending = append(pending, input.Raw)
335339
}
336340
}
337341
return pending, nil

internal/nix/input.go

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"go.jetpack.io/devbox/internal/boxcli/usererr"
2020
"go.jetpack.io/devbox/internal/cuecfg"
2121
"go.jetpack.io/devbox/internal/lock"
22-
"go.jetpack.io/devbox/internal/searcher"
2322
)
2423

2524
type Input struct {
@@ -108,10 +107,54 @@ func (i *Input) URLForInstall() (string, error) {
108107
return i.urlWithoutFragment() + "#" + attrPath, nil
109108
}
110109

111-
// PackageAttributePath returns just the name for non-flakes. For flake
112-
// references is returns the full path to the package in the flake. e.g.
113-
// packages.x86_64-linux.hello
110+
func (i *Input) normalizedDevboxPackageReference() (string, error) {
111+
if !i.IsDevboxPackage() {
112+
return "", nil
113+
}
114+
115+
path := ""
116+
if i.isVersioned() {
117+
entry, err := i.lockfile.Resolve(i.String())
118+
if err != nil {
119+
return "", err
120+
}
121+
path = entry.Resolved
122+
} else if i.IsDevboxPackage() {
123+
path = i.lockfile.LegacyNixpkgsPath(i.String())
124+
}
125+
126+
if path != "" {
127+
s, err := System()
128+
if err != nil {
129+
return "", err
130+
}
131+
url, fragment, _ := strings.Cut(path, "#")
132+
return fmt.Sprintf("%s#legacyPackages.%s.%s", url, s, fragment), nil
133+
}
134+
135+
return "", nil
136+
}
137+
138+
// PackageAttributePath returns the attribute path for a package. It is not
139+
// always normalized which means it should not be used to compare packages.
140+
// During happy paths (devbox packages and nix flakes that contains a fragment)
141+
// it is much faster than NormalizedPackageAttributePath
114142
func (i *Input) PackageAttributePath() (string, error) {
143+
if i.IsDevboxPackage() {
144+
reference, err := i.normalizedDevboxPackageReference()
145+
if err != nil {
146+
return "", err
147+
}
148+
_, fragment, _ := strings.Cut(reference, "#")
149+
return fragment, nil
150+
}
151+
return i.NormalizedPackageAttributePath()
152+
}
153+
154+
// NormalizedPackageAttributePath returns an attribute path normalized by nix
155+
// search. This is useful for comparing different attribute paths that may
156+
// point to the same package. Note, it's an expensive call.
157+
func (i *Input) NormalizedPackageAttributePath() (string, error) {
115158
var query string
116159
if i.isVersioned() {
117160
entry, err := i.lockfile.Resolve(i.String())
@@ -195,14 +238,10 @@ func (i *Input) hash() string {
195238
}
196239

197240
func (i *Input) ValidateExists() (bool, error) {
198-
if i.isVersioned() {
199-
version := i.version()
200-
if version == "" && i.isVersioned() {
201-
return false, usererr.New("No version specified for %q.", i.Path)
202-
}
203-
return searcher.Exists(i.CanonicalName(), version)
241+
if i.isVersioned() && i.version() == "" {
242+
return false, usererr.New("No version specified for %q.", i.Path)
204243
}
205-
info, err := i.PackageAttributePath()
244+
info, err := i.NormalizedPackageAttributePath()
206245
return info != "", err
207246
}
208247

@@ -216,11 +255,11 @@ func (i *Input) equals(other *Input) bool {
216255
return false
217256
}
218257

219-
name, err := i.PackageAttributePath()
258+
name, err := i.NormalizedPackageAttributePath()
220259
if err != nil {
221260
return false
222261
}
223-
otherName, err := other.PackageAttributePath()
262+
otherName, err := other.NormalizedPackageAttributePath()
224263
if err != nil {
225264
return false
226265
}

internal/nix/profile.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@ import (
2525

2626
const DefaultPriority = 5
2727

28-
type NixProfileList []*NixProfileListItem
29-
3028
// ProfileListItems returns a list of the installed packages
31-
func ProfileListItems(writer io.Writer, profileDir string) (NixProfileList, error) {
29+
func ProfileListItems(
30+
writer io.Writer,
31+
profileDir string,
32+
) (map[string]*NixProfileListItem, error) {
3233
cmd := exec.Command(
3334
"nix", "profile", "list",
3435
"--profile", profileDir,
@@ -55,7 +56,7 @@ func ProfileListItems(writer io.Writer, profileDir string) (NixProfileList, erro
5556
return nil, redact.Errorf("error starting \"nix profile list\" command: %w", err)
5657
}
5758

58-
items := []*NixProfileListItem{}
59+
items := map[string]*NixProfileListItem{}
5960
scanner := bufio.NewScanner(out)
6061
scanner.Split(bufio.ScanLines)
6162

@@ -66,7 +67,7 @@ func ProfileListItems(writer io.Writer, profileDir string) (NixProfileList, erro
6667
return nil, err
6768
}
6869

69-
items = append(items, item)
70+
items[item.unlockedReference] = item
7071
}
7172

7273
if err := cmd.Wait(); err != nil {
@@ -78,10 +79,10 @@ func ProfileListItems(writer io.Writer, profileDir string) (NixProfileList, erro
7879
type ProfileListIndexArgs struct {
7980
// For performance you can reuse the same list in multiple operations if you
8081
// are confident index has not changed.
81-
List NixProfileList
82+
List map[string]*NixProfileListItem
8283
Lockfile *lock.File
8384
Writer io.Writer
84-
Pkg string
85+
Input *Input
8586
ProfileDir string
8687
}
8788

@@ -95,12 +96,20 @@ func ProfileListIndex(args *ProfileListIndexArgs) (int, error) {
9596
}
9697
}
9798

98-
input := InputFromString(args.Pkg, args.Lockfile)
99+
// This is an optimization for happy path. A resolved devbox package
100+
// should match the unlockedReference of an existing profile item.
101+
ref, err := args.Input.normalizedDevboxPackageReference()
102+
if err != nil {
103+
return -1, err
104+
}
105+
if item, found := list[ref]; found {
106+
return item.index, nil
107+
}
99108

100109
for _, item := range list {
101110
existing := InputFromString(item.unlockedReference, args.Lockfile)
102111

103-
if input.equals(existing) {
112+
if args.Input.equals(existing) {
104113
return item.index, nil
105114
}
106115
}

0 commit comments

Comments
 (0)