perf(config): simplify loadFromConfigPaths (#1821)

Carlos Alexandro Becker created

Change summary

internal/config/attribution_migration_test.go |   4 
internal/config/load.go                       |  45 +++-----
internal/config/load_bench_test.go            | 103 +++++++++++++++++++++
internal/config/load_test.go                  |  11 +-
internal/config/merge.go                      |  16 ---
internal/config/merge_test.go                 |  27 -----
6 files changed, 126 insertions(+), 80 deletions(-)

Detailed changes

internal/config/attribution_migration_test.go 🔗

@@ -1,8 +1,6 @@
 package config
 
 import (
-	"io"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -83,7 +81,7 @@ func TestAttributionMigration(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			cfg, err := loadFromReaders([]io.Reader{strings.NewReader(tt.configJSON)})
+			cfg, err := loadFromBytes([][]byte{[]byte(tt.configJSON)})
 			require.NoError(t, err)
 
 			cfg.setDefaults(t.TempDir(), "")

internal/config/load.go 🔗

@@ -5,7 +5,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io"
 	"log/slog"
 	"maps"
 	"os"
@@ -25,25 +24,11 @@ import (
 	"github.com/charmbracelet/crush/internal/home"
 	"github.com/charmbracelet/crush/internal/log"
 	powernapConfig "github.com/charmbracelet/x/powernap/pkg/config"
+	"github.com/qjebbs/go-jsons"
 )
 
 const defaultCatwalkURL = "https://catwalk.charm.sh"
 
-// LoadReader config via io.Reader.
-func LoadReader(fd io.Reader) (*Config, error) {
-	data, err := io.ReadAll(fd)
-	if err != nil {
-		return nil, err
-	}
-
-	var config Config
-	err = json.Unmarshal(data, &config)
-	if err != nil {
-		return nil, err
-	}
-	return &config, err
-}
-
 // Load loads the configuration from the default paths.
 func Load(workingDir, dataDir string, debug bool) (*Config, error) {
 	configPaths := lookupConfigs(workingDir)
@@ -632,35 +617,39 @@ func lookupConfigs(cwd string) []string {
 }
 
 func loadFromConfigPaths(configPaths []string) (*Config, error) {
-	var configs []io.Reader
+	var configs [][]byte
 
 	for _, path := range configPaths {
-		fd, err := os.Open(path)
+		data, err := os.ReadFile(path)
 		if err != nil {
 			if os.IsNotExist(err) {
 				continue
 			}
 			return nil, fmt.Errorf("failed to open config file %s: %w", path, err)
 		}
-		defer fd.Close()
-
-		configs = append(configs, fd)
+		if len(data) == 0 {
+			continue
+		}
+		configs = append(configs, data)
 	}
 
-	return loadFromReaders(configs)
+	return loadFromBytes(configs)
 }
 
-func loadFromReaders(readers []io.Reader) (*Config, error) {
-	if len(readers) == 0 {
+func loadFromBytes(configs [][]byte) (*Config, error) {
+	if len(configs) == 0 {
 		return &Config{}, nil
 	}
 
-	merged, err := Merge(readers)
+	data, err := jsons.Merge(configs)
 	if err != nil {
-		return nil, fmt.Errorf("failed to merge configuration readers: %w", err)
+		return nil, err
 	}
-
-	return LoadReader(merged)
+	var config Config
+	if err := json.Unmarshal(data, &config); err != nil {
+		return nil, err
+	}
+	return &config, nil
 }
 
 func hasVertexCredentials(env env.Env) bool {

internal/config/load_bench_test.go 🔗

@@ -0,0 +1,103 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func BenchmarkLoadFromConfigPaths(b *testing.B) {
+	// Create temp config files with realistic content.
+	tmpDir := b.TempDir()
+
+	globalConfig := filepath.Join(tmpDir, "global.json")
+	localConfig := filepath.Join(tmpDir, "local.json")
+
+	globalContent := []byte(`{
+		"providers": {
+			"openai": {
+				"api_key": "$OPENAI_API_KEY",
+				"base_url": "https://api.openai.com/v1"
+			},
+			"anthropic": {
+				"api_key": "$ANTHROPIC_API_KEY",
+				"base_url": "https://api.anthropic.com"
+			}
+		},
+		"options": {
+			"tui": {
+				"theme": "dark"
+			}
+		}
+	}`)
+
+	localContent := []byte(`{
+		"providers": {
+			"openai": {
+				"api_key": "sk-override-key"
+			}
+		},
+		"options": {
+			"context_paths": ["README.md", "AGENTS.md"]
+		}
+	}`)
+
+	if err := os.WriteFile(globalConfig, globalContent, 0o644); err != nil {
+		b.Fatal(err)
+	}
+	if err := os.WriteFile(localConfig, localContent, 0o644); err != nil {
+		b.Fatal(err)
+	}
+
+	configPaths := []string{globalConfig, localConfig}
+
+	b.ReportAllocs()
+	for b.Loop() {
+		_, err := loadFromConfigPaths(configPaths)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkLoadFromConfigPaths_MissingFiles(b *testing.B) {
+	// Test with mix of existing and non-existing paths.
+	tmpDir := b.TempDir()
+
+	existingConfig := filepath.Join(tmpDir, "exists.json")
+	content := []byte(`{"options": {"tui": {"theme": "dark"}}}`)
+	if err := os.WriteFile(existingConfig, content, 0o644); err != nil {
+		b.Fatal(err)
+	}
+
+	configPaths := []string{
+		filepath.Join(tmpDir, "nonexistent1.json"),
+		existingConfig,
+		filepath.Join(tmpDir, "nonexistent2.json"),
+	}
+
+	b.ReportAllocs()
+	for b.Loop() {
+		_, err := loadFromConfigPaths(configPaths)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkLoadFromConfigPaths_Empty(b *testing.B) {
+	// Test with no config files.
+	tmpDir := b.TempDir()
+	configPaths := []string{
+		filepath.Join(tmpDir, "nonexistent1.json"),
+		filepath.Join(tmpDir, "nonexistent2.json"),
+	}
+
+	b.ReportAllocs()
+	for b.Loop() {
+		_, err := loadFromConfigPaths(configPaths)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}

internal/config/load_test.go 🔗

@@ -5,7 +5,6 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/charmbracelet/catwalk/pkg/catwalk"
@@ -22,12 +21,12 @@ func TestMain(m *testing.M) {
 	os.Exit(exitVal)
 }
 
-func TestConfig_LoadFromReaders(t *testing.T) {
-	data1 := strings.NewReader(`{"providers": {"openai": {"api_key": "key1", "base_url": "https://api.openai.com/v1"}}}`)
-	data2 := strings.NewReader(`{"providers": {"openai": {"api_key": "key2", "base_url": "https://api.openai.com/v2"}}}`)
-	data3 := strings.NewReader(`{"providers": {"openai": {}}}`)
+func TestConfig_LoadFromBytes(t *testing.T) {
+	data1 := []byte(`{"providers": {"openai": {"api_key": "key1", "base_url": "https://api.openai.com/v1"}}}`)
+	data2 := []byte(`{"providers": {"openai": {"api_key": "key2", "base_url": "https://api.openai.com/v2"}}}`)
+	data3 := []byte(`{"providers": {"openai": {}}}`)
 
-	loadedConfig, err := loadFromReaders([]io.Reader{data1, data2, data3})
+	loadedConfig, err := loadFromBytes([][]byte{data1, data2, data3})
 
 	require.NoError(t, err)
 	require.NotNil(t, loadedConfig)

internal/config/merge.go 🔗

@@ -1,16 +0,0 @@
-package config
-
-import (
-	"bytes"
-	"io"
-
-	"github.com/qjebbs/go-jsons"
-)
-
-func Merge(data []io.Reader) (io.Reader, error) {
-	got, err := jsons.Merge(data)
-	if err != nil {
-		return nil, err
-	}
-	return bytes.NewReader(got), nil
-}

internal/config/merge_test.go 🔗

@@ -1,27 +0,0 @@
-package config
-
-import (
-	"io"
-	"strings"
-	"testing"
-)
-
-func TestMerge(t *testing.T) {
-	data1 := strings.NewReader(`{"foo": "bar"}`)
-	data2 := strings.NewReader(`{"baz": "qux"}`)
-
-	merged, err := Merge([]io.Reader{data1, data2})
-	if err != nil {
-		t.Fatalf("expected no error, got %v", err)
-	}
-
-	expected := `{"foo":"bar","baz":"qux"}`
-	got, err := io.ReadAll(merged)
-	if err != nil {
-		t.Fatalf("expected no error reading merged data, got %v", err)
-	}
-
-	if string(got) != expected {
-		t.Errorf("expected %s, got %s", expected, string(got))
-	}
-}