From a447f55c5c3091d0d1dd406ea9e96e97aaaa12ff Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Mon, 12 Jan 2026 10:24:58 -0300 Subject: [PATCH] perf(config): simplify loadFromConfigPaths (#1821) --- 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(-) create mode 100644 internal/config/load_bench_test.go delete mode 100644 internal/config/merge.go delete mode 100644 internal/config/merge_test.go diff --git a/internal/config/attribution_migration_test.go b/internal/config/attribution_migration_test.go index 6c891d92a9a29604d9d6c751e0d15df6edcf3598..cc8c9e2e278b89ee2f86996975774abe18413843 100644 --- a/internal/config/attribution_migration_test.go +++ b/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(), "") diff --git a/internal/config/load.go b/internal/config/load.go index 63904abc057e877c959991769c20f62a7ac8459a..8323c07ac9d4d7d6cd801d617cc6c40da2377bba 100644 --- a/internal/config/load.go +++ b/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 { diff --git a/internal/config/load_bench_test.go b/internal/config/load_bench_test.go new file mode 100644 index 0000000000000000000000000000000000000000..3df43946d438fd478b63a1dfd242ee4c35e71896 --- /dev/null +++ b/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) + } + } +} diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 4819a195df5031a0e179903be013a89ca038791d..47cb0c5ec0ef8ae0266ed77b47fa60834d596399 100644 --- a/internal/config/load_test.go +++ b/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) diff --git a/internal/config/merge.go b/internal/config/merge.go deleted file mode 100644 index 3c9b7d6283a193166ad50730b28853a909f5158a..0000000000000000000000000000000000000000 --- a/internal/config/merge.go +++ /dev/null @@ -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 -} diff --git a/internal/config/merge_test.go b/internal/config/merge_test.go deleted file mode 100644 index 1b721bf2e8e4b4596025c2c773bec0093778f430..0000000000000000000000000000000000000000 --- a/internal/config/merge_test.go +++ /dev/null @@ -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)) - } -}