feat(comp): Speed up completion of charts

The completion of charts was using 'helm search repo' which can be
quite slow as it must parse the entire yaml of every repo cache file.
Using completion for a chart name can end up triggering multiple calls
to 'helm search'; this makes the user experience poor, as there is
a delay of over a second at every <TAB> press.

This commit creates a cache file for each repo which contains the list
of charts for that repo.  The completion logic then uses this new
cache file directly and obtains the chart names very quickly.

With only the stable repo configured, this optimization makes the
completion of charts about 85 times faster, going from 1.2 seconds to
0.014 seconds; such a difference gives a much better user experience
when completing chart names.

On the other hand, adding the creation of the chart list cache file to
'helm repo update' or 'helm repo add' is pretty much negligible compared
to the downloading of the index file.

It is also worth noting that when more repos are configured,
'helm search repo' only becomes slower, while the completion logic that
uses the new chart list cache file will not be affected as it only looks
for the single relevant repo file.

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>

cmd/helm/repo_add_test.go

@@ -19,6 +19,7 @@ package main
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"sync"
 	"testing"
@@ -26,6 +27,8 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"helm.sh/helm/v3/internal/test/ensure"
+	"helm.sh/helm/v3/pkg/helmpath"
+	"helm.sh/helm/v3/pkg/helmpath/xdg"
 	"helm.sh/helm/v3/pkg/repo"
 	"helm.sh/helm/v3/pkg/repo/repotest"
 )
@@ -55,7 +58,8 @@ func TestRepoAdd(t *testing.T) {
 	}
 	defer ts.Stop()
 
-	repoFile := filepath.Join(ensure.TempDir(t), "repositories.yaml")
+	rootDir := ensure.TempDir(t)
+	repoFile := filepath.Join(rootDir, "repositories.yaml")
 
 	const testRepoName = "test-name"
 
@@ -65,6 +69,7 @@ func TestRepoAdd(t *testing.T) {
 		noUpdate: true,
 		repoFile: repoFile,
 	}
+	os.Setenv(xdg.CacheHomeEnvVar, rootDir)
 
 	if err := o.run(ioutil.Discard); err != nil {
 		t.Error(err)
@@ -79,6 +84,15 @@ func TestRepoAdd(t *testing.T) {
 		t.Errorf("%s was not successfully inserted into %s", testRepoName, repoFile)
 	}
 
+	idx := filepath.Join(helmpath.CachePath("repository"), helmpath.CacheIndexFile(testRepoName))
+	if _, err := os.Stat(idx); os.IsNotExist(err) {
+		t.Errorf("Error cache index file was not created for repository %s", testRepoName)
+	}
+	idx = filepath.Join(helmpath.CachePath("repository"), helmpath.CacheChartsFile(testRepoName))
+	if _, err := os.Stat(idx); os.IsNotExist(err) {
+		t.Errorf("Error cache charts file was not created for repository %s", testRepoName)
+	}
+
 	o.noUpdate = false
 
 	if err := o.run(ioutil.Discard); err != nil {

cmd/helm/repo_remove.go

@@ -76,7 +76,12 @@ func (o *repoRemoveOptions) run(out io.Writer) error {
 }
 
 func removeRepoCache(root, name string) error {
-	idx := filepath.Join(root, helmpath.CacheIndexFile(name))
+	idx := filepath.Join(root, helmpath.CacheChartsFile(name))
+	if _, err := os.Stat(idx); err == nil {
+		os.Remove(idx)
+	}
+
+	idx = filepath.Join(root, helmpath.CacheIndexFile(name))
 	if _, err := os.Stat(idx); os.IsNotExist(err) {
 		return nil
 	} else if err != nil {

cmd/helm/repo_remove_test.go

@@ -63,10 +63,13 @@ func TestRepoRemove(t *testing.T) {
 	}
 
 	idx := filepath.Join(rootDir, helmpath.CacheIndexFile(testRepoName))
-
 	mf, _ := os.Create(idx)
 	mf.Close()
 
+	idx2 := filepath.Join(rootDir, helmpath.CacheChartsFile(testRepoName))
+	mf, _ = os.Create(idx2)
+	mf.Close()
+
 	b.Reset()
 
 	if err := rmOpts.run(b); err != nil {
@@ -77,7 +80,11 @@ func TestRepoRemove(t *testing.T) {
 	}
 
 	if _, err := os.Stat(idx); err == nil {
-		t.Errorf("Error cache file was not removed for repository %s", testRepoName)
+		t.Errorf("Error cache index file was not removed for repository %s", testRepoName)
+	}
+
+	if _, err := os.Stat(idx2); err == nil {
+		t.Errorf("Error cache chart file was not removed for repository %s", testRepoName)
 	}
 
 	f, err := repo.LoadFile(repoFile)

cmd/helm/root.go

@@ -139,13 +139,25 @@ __helm_zsh_comp_nospace() {
 __helm_list_charts()
 {
     __helm_debug "${FUNCNAME[0]}: c is $c words[c] is ${words[c]}"
-    local repo url file out=() nospace=0 wantFiles=$1
+    local prefix chart repo url file out=() nospace=0 wantFiles=$1
 
     # Handle completions for repos
     for repo in $(__helm_get_repos); do
         if [[ "${cur}" =~ ^${repo}/.* ]]; then
             # We are doing completion from within a repo
-            out=$(eval $(__helm_binary_name) search repo ${cur} 2>/dev/null | \cut -f1 | \grep ^${cur})
+            local cacheFile=$(eval $(__helm_binary_name) env 2>/dev/null | \grep HELM_REPOSITORY_CACHE | \cut -d= -f2 | \sed s/\"//g)/${repo}-charts.txt
+            if [ -f "$cacheFile" ]; then
+                # Get the list of charts from the cached file
+                prefix=${cur#${repo}/}
+                for chart in $(\grep ^$prefix $cacheFile); do
+                    out+=(${repo}/${chart})
+                done
+            else
+                # If there is no cached list file, fallback to helm search, which is much slower
+                # This will happen after the caching feature is first installed but before the user
+                # does a 'helm repo update' to generate the cached list file.
+				out=$(eval $(__helm_binary_name) search repo ${cur} 2>/dev/null | \cut -f1 | \grep ^${cur})
+            fi
             nospace=0
         elif [[ ${repo} =~ ^${cur}.* ]]; then
             # We are completing a repo name

pkg/helmpath/home.go

@@ -25,10 +25,19 @@ func CachePath(elem ...string) string { return lp.cachePath(elem...) }
 // DataPath returns the path where Helm stores data.
 func DataPath(elem ...string) string { return lp.dataPath(elem...) }
 
-// CacheIndex returns the path to an index for the given named repository.
+// CacheIndexFile returns the path to an index for the given named repository.
 func CacheIndexFile(name string) string {
 	if name != "" {
 		name += "-"
 	}
 	return name + "index.yaml"
 }
+
+// CacheChartsFile returns the path to a text file listing all the charts
+// within the given named repository.
+func CacheChartsFile(name string) string {
+	if name != "" {
+		name += "-"
+	}
+	return name + "charts.txt"
+}

pkg/repo/chartrepo.go

@@ -133,10 +133,21 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) {
 		return "", err
 	}
 
-	if _, err := loadIndex(index); err != nil {
+	indexFile, err := loadIndex(index)
+	if err != nil {
 		return "", err
 	}
 
+	// Create the chart list file in the cache directory
+	var charts strings.Builder
+	for name := range indexFile.Entries {
+		fmt.Fprintln(&charts, name)
+	}
+	chartsFile := filepath.Join(r.CachePath, helmpath.CacheChartsFile(r.Config.Name))
+	os.MkdirAll(filepath.Dir(chartsFile), 0755)
+	ioutil.WriteFile(chartsFile, []byte(charts.String()), 0644)
+
+	// Create the index file in the cache directory
 	fname := filepath.Join(r.CachePath, helmpath.CacheIndexFile(r.Config.Name))
 	os.MkdirAll(filepath.Dir(fname), 0755)
 	return fname, ioutil.WriteFile(fname, index, 0644)

pkg/repo/index_test.go

@@ -17,14 +17,19 @@ limitations under the License.
 package repo
 
 import (
+	"bufio"
+	"bytes"
 	"io/ioutil"
 	"net/http"
 	"os"
+	"path/filepath"
+	"sort"
 	"strings"
 	"testing"
 
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/getter"
+	"helm.sh/helm/v3/pkg/helmpath"
 
 	"helm.sh/helm/v3/pkg/chart"
 )
@@ -178,6 +183,18 @@ func TestDownloadIndexFile(t *testing.T) {
 			t.Fatalf("Index %q failed to parse: %s", testfile, err)
 		}
 		verifyLocalIndex(t, i)
+
+		// Check that charts file is also created
+		idx = filepath.Join(r.CachePath, helmpath.CacheChartsFile(r.Config.Name))
+		if _, err := os.Stat(idx); err != nil {
+			t.Fatalf("error finding created charts file: %#v", err)
+		}
+
+		b, err = ioutil.ReadFile(idx)
+		if err != nil {
+			t.Fatalf("error reading charts file: %#v", err)
+		}
+		verifyLocalChartsFile(t, b, i)
 	})
 
 	t.Run("should not decode the path in the repo url while downloading index", func(t *testing.T) {
@@ -224,6 +241,18 @@ func TestDownloadIndexFile(t *testing.T) {
 			t.Fatalf("Index %q failed to parse: %s", testfile, err)
 		}
 		verifyLocalIndex(t, i)
+
+		// Check that charts file is also created
+		idx = filepath.Join(r.CachePath, helmpath.CacheChartsFile(r.Config.Name))
+		if _, err := os.Stat(idx); err != nil {
+			t.Fatalf("error finding created charts file: %#v", err)
+		}
+
+		b, err = ioutil.ReadFile(idx)
+		if err != nil {
+			t.Fatalf("error reading charts file: %#v", err)
+		}
+		verifyLocalChartsFile(t, b, i)
 	})
 }
 
@@ -322,6 +351,24 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) {
 	}
 }
 
+func verifyLocalChartsFile(t *testing.T, chartsContent []byte, indexContent *IndexFile) {
+	var expected, real []string
+	for chart := range indexContent.Entries {
+		expected = append(expected, chart)
+	}
+	sort.Strings(expected)
+
+	scanner := bufio.NewScanner(bytes.NewReader(chartsContent))
+	for scanner.Scan() {
+		real = append(real, scanner.Text())
+	}
+	sort.Strings(real)
+
+	if strings.Join(expected, " ") != strings.Join(real, " ") {
+		t.Errorf("Cached charts file content unexpected. Expected:\n%s\ngot:\n%s", expected, real)
+	}
+}
+
 func TestIndexDirectory(t *testing.T) {
 	dir := "testdata/repository"
 	index, err := IndexDirectory(dir, "http://localhost:8080")