a447f55 · crush · git.iko.soy

git.iko.soy/crush

on main help sign in

perf(config): simplify loadFromConfigPaths (#1821)

a447f55Carlos Alexandro Becker9 months ago

internal/config/attribution_migration_test.go+1−3

compare with main

unifiedsplit

@@ -1,8 +1,6 @@

1package config

3import (

4− "io"

5− "strings"

6 "testing"

4 "testing"

8 "github.com/stretchr/testify/require"

6 "github.com/stretchr/testify/require"

@@ -83,7 +81,7 @@ func TestAttributionMigration(t *testing.T) {

83 t.Run(tt.name, func(t *testing.T) {

81 t.Run(tt.name, func(t *testing.T) {

84 t.Parallel()

82 t.Parallel()

86− cfg, err := loadFromReaders([]io.Reader{strings.NewReader(tt.configJSON)})

84+ cfg, err := loadFromBytes([][]byte{[]byte(tt.configJSON)})

87 require.NoError(t, err)

85 require.NoError(t, err)

89 cfg.setDefaults(t.TempDir(), "")

87 cfg.setDefaults(t.TempDir(), "")

internal/config/load.go+17−28

@@ -5,7 +5,6 @@ import (

5 "context"

6 "encoding/json"

7 "fmt"

8− "io"

9 "log/slog"

8 "log/slog"

10 "maps"

9 "maps"

11 "os"

10 "os"

@@ -25,25 +24,11 @@ import (

25 "github.com/charmbracelet/crush/internal/home"

24 "github.com/charmbracelet/crush/internal/home"

26 "github.com/charmbracelet/crush/internal/log"

25 "github.com/charmbracelet/crush/internal/log"

27 powernapConfig "github.com/charmbracelet/x/powernap/pkg/config"

26 powernapConfig "github.com/charmbracelet/x/powernap/pkg/config"

27+ "github.com/qjebbs/go-jsons"

28)

30const defaultCatwalkURL = "https://catwalk.charm.sh"

32−// LoadReader config via io.Reader.

33−func LoadReader(fd io.Reader) (*Config, error) {

34− data, err := io.ReadAll(fd)

35− if err != nil {

36− return nil, err

37− }

38−

39− var config Config

40− err = json.Unmarshal(data, &config)

41− if err != nil {

42− return nil, err

43− }

44− return &config, err

45−}

46−

47// Load loads the configuration from the default paths.

32// Load loads the configuration from the default paths.

48func Load(workingDir, dataDir string, debug bool) (*Config, error) {

33func Load(workingDir, dataDir string, debug bool) (*Config, error) {

49 configPaths := lookupConfigs(workingDir)

34 configPaths := lookupConfigs(workingDir)

@@ -632,35 +617,39 @@ func lookupConfigs(cwd string) []string {

632}

617}

633

618

634func loadFromConfigPaths(configPaths []string) (*Config, error) {

619func loadFromConfigPaths(configPaths []string) (*Config, error) {

635− var configs []io.Reader

620+ var configs [][]byte

636

621

637 for _, path := range configPaths {

622 for _, path := range configPaths {

638− fd, err := os.Open(path)

623+ data, err := os.ReadFile(path)

639 if err != nil {

624 if err != nil {

640 if os.IsNotExist(err) {

625 if os.IsNotExist(err) {

641 continue

626 continue

642 }

627 }

643 return nil, fmt.Errorf("failed to open config file %s: %w", path, err)

628 return nil, fmt.Errorf("failed to open config file %s: %w", path, err)

644 }

629 }

645− defer fd.Close()

630+ if len(data) == 0 {

646−

631+ continue

632+ }

647− configs = append(configs, fd)

633+ configs = append(configs, data)

648 }

634 }

649

635

650− return loadFromReaders(configs)

636+ return loadFromBytes(configs)

651}

637}

652

638

653−func loadFromReaders(readers []io.Reader) (*Config, error) {

639+func loadFromBytes(configs [][]byte) (*Config, error) {

654− if len(readers) == 0 {

640+ if len(configs) == 0 {

655 return &Config{}, nil

641 return &Config{}, nil

656 }

642 }

657

643

658− merged, err := Merge(readers)

644+ data, err := jsons.Merge(configs)

659 if err != nil {

645 if err != nil {

660− return nil, fmt.Errorf("failed to merge configuration readers: %w", err)

646+ return nil, err

661 }

647 }

662−

648+ var config Config

663− return LoadReader(merged)

649+ if err := json.Unmarshal(data, &config); err != nil {

650+ return nil, err

651+ }

652+ return &config, nil

664}

653}

665

654

666func hasVertexCredentials(env env.Env) bool {

655func hasVertexCredentials(env env.Env) bool {

internal/config/load_bench_test.gonew file+103

@@ -0,0 +1,103 @@

1+package config

3+import (

4+ "os"

5+ "path/filepath"

6+ "testing"

7+)

9+func BenchmarkLoadFromConfigPaths(b *testing.B) {

10+ // Create temp config files with realistic content.

11+ tmpDir := b.TempDir()

12+

13+ globalConfig := filepath.Join(tmpDir, "global.json")

14+ localConfig := filepath.Join(tmpDir, "local.json")

15+

16+ globalContent := []byte(`{

17+ "providers": {

18+ "openai": {

19+ "api_key": "$OPENAI_API_KEY",

20+ "base_url": "https://api.openai.com/v1"

21+ },

22+ "anthropic": {

23+ "api_key": "$ANTHROPIC_API_KEY",

24+ "base_url": "https://api.anthropic.com"

25+ }

26+ },

27+ "options": {

28+ "tui": {

29+ "theme": "dark"

30+ }

31+ }

32+ }`)

33+

34+ localContent := []byte(`{

35+ "providers": {

36+ "openai": {

37+ "api_key": "sk-override-key"

38+ }

39+ },

40+ "options": {

41+ "context_paths": ["README.md", "AGENTS.md"]

42+ }

43+ }`)

44+

45+ if err := os.WriteFile(globalConfig, globalContent, 0o644); err != nil {

46+ b.Fatal(err)

47+ }

48+ if err := os.WriteFile(localConfig, localContent, 0o644); err != nil {

49+ b.Fatal(err)

50+ }

51+

52+ configPaths := []string{globalConfig, localConfig}

53+

54+ b.ReportAllocs()

55+ for b.Loop() {

56+ _, err := loadFromConfigPaths(configPaths)

57+ if err != nil {

58+ b.Fatal(err)

59+ }

60+ }

61+}

62+

63+func BenchmarkLoadFromConfigPaths_MissingFiles(b *testing.B) {

64+ // Test with mix of existing and non-existing paths.

65+ tmpDir := b.TempDir()

66+

67+ existingConfig := filepath.Join(tmpDir, "exists.json")

68+ content := []byte(`{"options": {"tui": {"theme": "dark"}}}`)

69+ if err := os.WriteFile(existingConfig, content, 0o644); err != nil {

70+ b.Fatal(err)

71+ }

72+

73+ configPaths := []string{

74+ filepath.Join(tmpDir, "nonexistent1.json"),

75+ existingConfig,

76+ filepath.Join(tmpDir, "nonexistent2.json"),

77+ }

78+

79+ b.ReportAllocs()

80+ for b.Loop() {

81+ _, err := loadFromConfigPaths(configPaths)

82+ if err != nil {

83+ b.Fatal(err)

84+ }

85+ }

86+}

87+

88+func BenchmarkLoadFromConfigPaths_Empty(b *testing.B) {

89+ // Test with no config files.

90+ tmpDir := b.TempDir()

91+ configPaths := []string{

92+ filepath.Join(tmpDir, "nonexistent1.json"),

93+ filepath.Join(tmpDir, "nonexistent2.json"),

94+ }

95+

96+ b.ReportAllocs()

97+ for b.Loop() {

98+ _, err := loadFromConfigPaths(configPaths)

99+ if err != nil {

100+ b.Fatal(err)

101+ }

102+ }

103+}

internal/config/load_test.go+5−6

@@ -5,7 +5,6 @@ import (

5 "log/slog"

6 "os"

7 "path/filepath"

8− "strings"

9 "testing"

8 "testing"

11 "github.com/charmbracelet/catwalk/pkg/catwalk"

10 "github.com/charmbracelet/catwalk/pkg/catwalk"

@@ -22,12 +21,12 @@ func TestMain(m *testing.M) {

22 os.Exit(exitVal)

21 os.Exit(exitVal)

23}

22}

25−func TestConfig_LoadFromReaders(t *testing.T) {

24+func TestConfig_LoadFromBytes(t *testing.T) {

26− data1 := strings.NewReader(`{"providers": {"openai": {"api_key": "key1", "base_url": "https://api.openai.com/v1"}}}`)

25+ data1 := []byte(`{"providers": {"openai": {"api_key": "key1", "base_url": "https://api.openai.com/v1"}}}`)

27− data2 := strings.NewReader(`{"providers": {"openai": {"api_key": "key2", "base_url": "https://api.openai.com/v2"}}}`)

26+ data2 := []byte(`{"providers": {"openai": {"api_key": "key2", "base_url": "https://api.openai.com/v2"}}}`)

28− data3 := strings.NewReader(`{"providers": {"openai": {}}}`)

27+ data3 := []byte(`{"providers": {"openai": {}}}`)

30− loadedConfig, err := loadFromReaders([]io.Reader{data1, data2, data3})

29+ loadedConfig, err := loadFromBytes([][]byte{data1, data2, data3})

32 require.NoError(t, err)

31 require.NoError(t, err)

33 require.NotNil(t, loadedConfig)

32 require.NotNil(t, loadedConfig)

internal/config/merge.godeleted−16

@@ -1,16 +0,0 @@

1−package config

2−

3−import (

4− "bytes"

5− "io"

6−

7− "github.com/qjebbs/go-jsons"

8−)

9−

10−func Merge(data []io.Reader) (io.Reader, error) {

11− got, err := jsons.Merge(data)

12− if err != nil {

13− return nil, err

14− }

15− return bytes.NewReader(got), nil

16−}

internal/config/merge_test.godeleted−27

@@ -1,27 +0,0 @@

1−package config

2−

3−import (

4− "io"

5− "strings"

6− "testing"

7−)

8−

9−func TestMerge(t *testing.T) {

10− data1 := strings.NewReader(`{"foo": "bar"}`)

11− data2 := strings.NewReader(`{"baz": "qux"}`)

12−

13− merged, err := Merge([]io.Reader{data1, data2})

14− if err != nil {

15− t.Fatalf("expected no error, got %v", err)

16− }

17−

18− expected := `{"foo":"bar","baz":"qux"}`

19− got, err := io.ReadAll(merged)

20− if err != nil {

21− t.Fatalf("expected no error reading merged data, got %v", err)

22− }

23−

24− if string(got) != expected {

25− t.Errorf("expected %s, got %s", expected, string(got))

26− }

27−}