on main help sign in
refactor: use csync.Map instead of sync.Map
commitb92d8800e3451a908bcb568bc97d9a3fb5a53232 authorCarlos Alexandro Becker <caarlos0@users.noreply.github.com> dateFri 25 Jul 2025, 11:25 (1 year ago) parent4a40997
6 files changed+314−84
internal/config/config.go+1−1 internal/csync/maps.go+24 internal/csync/maps_test.go+225 internal/llm/agent/agent.go+22−32 internal/permission/permission.go+11−9 internal/tui/components/chat/sidebar/sidebar.go+31−42
internal/config/config.go+1−1
@@ -270,7 +270,7 @@ func (c *Config) WorkingDir() string {
270270
271271func (c *Config) EnabledProviders() []ProviderConfig {
272272 var enabled []ProviderConfig
273− for _, p := range c.Providers.Seq2() {
273+ for p := range c.Providers.Seq() {
274274 if !p.Disable {
275275 enabled = append(enabled, p)
276276 }
internal/csync/maps.go+24
@@ -56,6 +56,15 @@ func (m *Map[K, V]) Len() int {
5656 return len(m.inner)
5757}
5858
59+// Take gets an item and then deletes it.
60+func (m *Map[K, V]) Take(key K) (V, bool) {
61+ m.mu.Lock()
62+ defer m.mu.Unlock()
63+ v, ok := m.inner[key]
64+ delete(m.inner, key)
65+ return v, ok
66+}
67+
5968// Seq2 returns an iter.Seq2 that yields key-value pairs from the map.
6069func (m *Map[K, V]) Seq2() iter.Seq2[K, V] {
6170 dst := make(map[K]V)
@@ -71,6 +80,21 @@ func (m *Map[K, V]) Seq2() iter.Seq2[K, V] {
7180 }
7281}
7382
83+// Seq returns an iter.Seq that yields values from the map.
84+func (m *Map[K, V]) Seq() iter.Seq[V] {
85+ dst := make(map[K]V)
86+ m.mu.RLock()
87+ maps.Copy(dst, m.inner)
88+ m.mu.RUnlock()
89+ return func(yield func(V) bool) {
90+ for _, v := range dst {
91+ if !yield(v) {
92+ return
93+ }
94+ }
95+ }
96+}
97+
7498var (
7599 _ json.Unmarshaler = &Map[string, any]{}
76100 _ json.Marshaler = &Map[string, any]{}
internal/csync/maps_test.go+225
@@ -110,6 +110,72 @@ func TestMap_Len(t *testing.T) {
110110 assert.Equal(t, 0, m.Len())
111111}
112112
113+func TestMap_Take(t *testing.T) {
114+ t.Parallel()
115+
116+ m := NewMap[string, int]()
117+ m.Set("key1", 42)
118+ m.Set("key2", 100)
119+
120+ assert.Equal(t, 2, m.Len())
121+
122+ value, ok := m.Take("key1")
123+ assert.True(t, ok)
124+ assert.Equal(t, 42, value)
125+ assert.Equal(t, 1, m.Len())
126+
127+ _, exists := m.Get("key1")
128+ assert.False(t, exists)
129+
130+ value, ok = m.Get("key2")
131+ assert.True(t, ok)
132+ assert.Equal(t, 100, value)
133+}
134+
135+func TestMap_Take_NonexistentKey(t *testing.T) {
136+ t.Parallel()
137+
138+ m := NewMap[string, int]()
139+ m.Set("key1", 42)
140+
141+ value, ok := m.Take("nonexistent")
142+ assert.False(t, ok)
143+ assert.Equal(t, 0, value)
144+ assert.Equal(t, 1, m.Len())
145+
146+ value, ok = m.Get("key1")
147+ assert.True(t, ok)
148+ assert.Equal(t, 42, value)
149+}
150+
151+func TestMap_Take_EmptyMap(t *testing.T) {
152+ t.Parallel()
153+
154+ m := NewMap[string, int]()
155+
156+ value, ok := m.Take("key1")
157+ assert.False(t, ok)
158+ assert.Equal(t, 0, value)
159+ assert.Equal(t, 0, m.Len())
160+}
161+
162+func TestMap_Take_SameKeyTwice(t *testing.T) {
163+ t.Parallel()
164+
165+ m := NewMap[string, int]()
166+ m.Set("key1", 42)
167+
168+ value, ok := m.Take("key1")
169+ assert.True(t, ok)
170+ assert.Equal(t, 42, value)
171+ assert.Equal(t, 0, m.Len())
172+
173+ value, ok = m.Take("key1")
174+ assert.False(t, ok)
175+ assert.Equal(t, 0, value)
176+ assert.Equal(t, 0, m.Len())
177+}
178+
113179func TestMap_Seq2(t *testing.T) {
114180 t.Parallel()
115181
@@ -158,6 +224,57 @@ func TestMap_Seq2_EmptyMap(t *testing.T) {
158224 assert.Equal(t, 0, count)
159225}
160226
227+func TestMap_Seq(t *testing.T) {
228+ t.Parallel()
229+
230+ m := NewMap[string, int]()
231+ m.Set("key1", 1)
232+ m.Set("key2", 2)
233+ m.Set("key3", 3)
234+
235+ collected := make([]int, 0)
236+ for v := range m.Seq() {
237+ collected = append(collected, v)
238+ }
239+
240+ assert.Equal(t, 3, len(collected))
241+ assert.Contains(t, collected, 1)
242+ assert.Contains(t, collected, 2)
243+ assert.Contains(t, collected, 3)
244+}
245+
246+func TestMap_Seq_EarlyReturn(t *testing.T) {
247+ t.Parallel()
248+
249+ m := NewMap[string, int]()
250+ m.Set("key1", 1)
251+ m.Set("key2", 2)
252+ m.Set("key3", 3)
253+
254+ count := 0
255+ for range m.Seq() {
256+ count++
257+ if count == 2 {
258+ break
259+ }
260+ }
261+
262+ assert.Equal(t, 2, count)
263+}
264+
265+func TestMap_Seq_EmptyMap(t *testing.T) {
266+ t.Parallel()
267+
268+ m := NewMap[string, int]()
269+
270+ count := 0
271+ for range m.Seq() {
272+ count++
273+ }
274+
275+ assert.Equal(t, 0, count)
276+}
277+
161278func TestMap_MarshalJSON(t *testing.T) {
162279 t.Parallel()
163280
@@ -371,6 +488,82 @@ func TestMap_ConcurrentSeq2(t *testing.T) {
371488 wg.Wait()
372489}
373490
491+func TestMap_ConcurrentSeq(t *testing.T) {
492+ t.Parallel()
493+
494+ m := NewMap[int, int]()
495+ for i := range 100 {
496+ m.Set(i, i*2)
497+ }
498+
499+ var wg sync.WaitGroup
500+ const numIterators = 10
501+
502+ wg.Add(numIterators)
503+ for range numIterators {
504+ go func() {
505+ defer wg.Done()
506+ count := 0
507+ values := make(map[int]bool)
508+ for v := range m.Seq() {
509+ values[v] = true
510+ count++
511+ }
512+ assert.Equal(t, 100, count)
513+ for i := range 100 {
514+ assert.True(t, values[i*2])
515+ }
516+ }()
517+ }
518+
519+ wg.Wait()
520+}
521+
522+func TestMap_ConcurrentTake(t *testing.T) {
523+ t.Parallel()
524+
525+ m := NewMap[int, int]()
526+ const numItems = 1000
527+
528+ for i := range numItems {
529+ m.Set(i, i*2)
530+ }
531+
532+ var wg sync.WaitGroup
533+ const numWorkers = 10
534+ taken := make([][]int, numWorkers)
535+
536+ wg.Add(numWorkers)
537+ for i := range numWorkers {
538+ go func(workerID int) {
539+ defer wg.Done()
540+ taken[workerID] = make([]int, 0)
541+ for j := workerID; j < numItems; j += numWorkers {
542+ if value, ok := m.Take(j); ok {
543+ taken[workerID] = append(taken[workerID], value)
544+ }
545+ }
546+ }(i)
547+ }
548+
549+ wg.Wait()
550+
551+ assert.Equal(t, 0, m.Len())
552+
553+ allTaken := make(map[int]bool)
554+ for _, workerTaken := range taken {
555+ for _, value := range workerTaken {
556+ assert.False(t, allTaken[value], "Value %d was taken multiple times", value)
557+ allTaken[value] = true
558+ }
559+ }
560+
561+ assert.Equal(t, numItems, len(allTaken))
562+ for i := range numItems {
563+ assert.True(t, allTaken[i*2], "Expected value %d to be taken", i*2)
564+ }
565+}
566+
374567func TestMap_TypeSafety(t *testing.T) {
375568 t.Parallel()
376569
@@ -431,6 +624,38 @@ func BenchmarkMap_Seq2(b *testing.B) {
431624 }
432625}
433626
627+func BenchmarkMap_Seq(b *testing.B) {
628+ m := NewMap[int, int]()
629+ for i := range 1000 {
630+ m.Set(i, i*2)
631+ }
632+
633+ for b.Loop() {
634+ for range m.Seq() {
635+ }
636+ }
637+}
638+
639+func BenchmarkMap_Take(b *testing.B) {
640+ m := NewMap[int, int]()
641+ for i := range 1000 {
642+ m.Set(i, i*2)
643+ }
644+
645+ b.ResetTimer()
646+ for i := 0; b.Loop(); i++ {
647+ key := i % 1000
648+ m.Take(key)
649+ if i%1000 == 999 {
650+ b.StopTimer()
651+ for j := range 1000 {
652+ m.Set(j, j*2)
653+ }
654+ b.StartTimer()
655+ }
656+ }
657+}
658+
434659func BenchmarkMap_ConcurrentReadWrite(b *testing.B) {
435660 m := NewMap[int, int]()
436661 for i := range 1000 {
internal/llm/agent/agent.go+22−32
@@ -7,7 +7,6 @@ import (
77 "log/slog"
88 "slices"
99 "strings"
10− "sync"
1110 "time"
1211
1312 "github.com/charmbracelet/catwalk/pkg/catwalk"
@@ -78,7 +77,7 @@ type agent struct {
7877 summarizeProvider provider.Provider
7978 summarizeProviderID string
8079
81− activeRequests sync.Map
80+ activeRequests *csync.Map[string, context.CancelFunc]
8281}
8382
8483var agentPromptMap = map[string]prompt.PromptID{
@@ -222,7 +221,7 @@ func NewAgent(
222221 titleProvider: titleProvider,
223222 summarizeProvider: summarizeProvider,
224223 summarizeProviderID: string(smallModelProviderCfg.ID),
225− activeRequests: sync.Map{},
224+ activeRequests: csync.NewMap[string, context.CancelFunc](),
226225 tools: csync.NewLazySlice(toolFn),
227226 }, nil
228227}
@@ -233,38 +232,30 @@ func (a *agent) Model() catwalk.Model {
233232
234233func (a *agent) Cancel(sessionID string) {
235234 // Cancel regular requests
236− if cancelFunc, exists := a.activeRequests.LoadAndDelete(sessionID); exists {
237− if cancel, ok := cancelFunc.(context.CancelFunc); ok {
238− slog.Info("Request cancellation initiated", "session_id", sessionID)
239− cancel()
240− }
235+ if cancel, ok := a.activeRequests.Take(sessionID); ok && cancel != nil {
236+ slog.Info("Request cancellation initiated", "session_id", sessionID)
237+ cancel()
241238 }
242239
243240 // Also check for summarize requests
244− if cancelFunc, exists := a.activeRequests.LoadAndDelete(sessionID + "-summarize"); exists {
245− if cancel, ok := cancelFunc.(context.CancelFunc); ok {
246− slog.Info("Summarize cancellation initiated", "session_id", sessionID)
247− cancel()
248− }
241+ if cancel, ok := a.activeRequests.Take(sessionID + "-summarize"); ok && cancel != nil {
242+ slog.Info("Summarize cancellation initiated", "session_id", sessionID)
243+ cancel()
249244 }
250245}
251246
252247func (a *agent) IsBusy() bool {
253− busy := false
254− a.activeRequests.Range(func(key, value any) bool {
255− if cancelFunc, ok := value.(context.CancelFunc); ok {
256− if cancelFunc != nil {
257− busy = true
258− return false
259− }
248+ var busy bool
249+ for cancelFunc := range a.activeRequests.Seq() {
250+ if cancelFunc != nil {
251+ busy = true
260252 }
261− return true
262− })
253+ }
263254 return busy
264255}
265256
266257func (a *agent) IsSessionBusy(sessionID string) bool {
267− _, busy := a.activeRequests.Load(sessionID)
258+ _, busy := a.activeRequests.Get(sessionID)
268259 return busy
269260}
270261
@@ -335,7 +326,7 @@ func (a *agent) Run(ctx context.Context, sessionID string, content string, attac
335326
336327 genCtx, cancel := context.WithCancel(ctx)
337328
338− a.activeRequests.Store(sessionID, cancel)
329+ a.activeRequests.Set(sessionID, cancel)
339330 go func() {
340331 slog.Debug("Request started", "sessionID", sessionID)
341332 defer log.RecoverPanic("agent.Run", func() {
@@ -350,7 +341,7 @@ func (a *agent) Run(ctx context.Context, sessionID string, content string, attac
350341 slog.Error(result.Error.Error())
351342 }
352343 slog.Debug("Request completed", "sessionID", sessionID)
353− a.activeRequests.Delete(sessionID)
344+ a.activeRequests.Del(sessionID)
354345 cancel()
355346 a.Publish(pubsub.CreatedEvent, result)
356347 events <- result
@@ -682,10 +673,10 @@ func (a *agent) Summarize(ctx context.Context, sessionID string) error {
682673 summarizeCtx, cancel := context.WithCancel(ctx)
683674
684675 // Store the cancel function in activeRequests to allow cancellation
685− a.activeRequests.Store(sessionID+"-summarize", cancel)
676+ a.activeRequests.Set(sessionID+"-summarize", cancel)
686677
687678 go func() {
688− defer a.activeRequests.Delete(sessionID + "-summarize")
679+ defer a.activeRequests.Del(sessionID + "-summarize")
689680 defer cancel()
690681 event := AgentEvent{
691682 Type: AgentEventTypeSummarize,
@@ -850,10 +841,9 @@ func (a *agent) CancelAll() {
850841 if !a.IsBusy() {
851842 return
852843 }
853− a.activeRequests.Range(func(key, value any) bool {
854− a.Cancel(key.(string)) // key is sessionID
855− return true
856− })
844+ for key := range a.activeRequests.Seq2() {
845+ a.Cancel(key) // key is sessionID
846+ }
857847
858848 timeout := time.After(5 * time.Second)
859849 for a.IsBusy() {
@@ -907,7 +897,7 @@ func (a *agent) UpdateModel() error {
907897 smallModelCfg := cfg.Models[config.SelectedModelTypeSmall]
908898 var smallModelProviderCfg config.ProviderConfig
909899
910− for _, p := range cfg.Providers.Seq2() {
900+ for p := range cfg.Providers.Seq() {
911901 if p.ID == smallModelCfg.Provider {
912902 smallModelProviderCfg = p
913903 break
internal/permission/permission.go+11−9
@@ -6,6 +6,7 @@ import (
66 "slices"
77 "sync"
88
9+ "github.com/charmbracelet/crush/internal/csync"
910 "github.com/charmbracelet/crush/internal/pubsub"
1011 "github.com/google/uuid"
1112)
@@ -46,7 +47,7 @@ type permissionService struct {
4647 workingDir string
4748 sessionPermissions []PermissionRequest
4849 sessionPermissionsMu sync.RWMutex
49− pendingRequests sync.Map
50+ pendingRequests *csync.Map[string, chan bool]
5051 autoApproveSessions []string
5152 autoApproveSessionsMu sync.RWMutex
5253 skip bool
@@ -54,9 +55,9 @@ type permissionService struct {
5455}
5556
5657func (s *permissionService) GrantPersistent(permission PermissionRequest) {
57− respCh, ok := s.pendingRequests.Load(permission.ID)
58+ respCh, ok := s.pendingRequests.Get(permission.ID)
5859 if ok {
59− respCh.(chan bool) <- true
60+ respCh <- true
6061 }
6162
6263 s.sessionPermissionsMu.Lock()
@@ -65,16 +66,16 @@ func (s *permissionService) GrantPersistent(permission PermissionRequest) {
6566}
6667
6768func (s *permissionService) Grant(permission PermissionRequest) {
68− respCh, ok := s.pendingRequests.Load(permission.ID)
69+ respCh, ok := s.pendingRequests.Get(permission.ID)
6970 if ok {
70− respCh.(chan bool) <- true
71+ respCh <- true
7172 }
7273}
7374
7475func (s *permissionService) Deny(permission PermissionRequest) {
75− respCh, ok := s.pendingRequests.Load(permission.ID)
76+ respCh, ok := s.pendingRequests.Get(permission.ID)
7677 if ok {
77− respCh.(chan bool) <- false
78+ respCh <- false
7879 }
7980}
8081
@@ -122,8 +123,8 @@ func (s *permissionService) Request(opts CreatePermissionRequest) bool {
122123
123124 respCh := make(chan bool, 1)
124125
125− s.pendingRequests.Store(permission.ID, respCh)
126− defer s.pendingRequests.Delete(permission.ID)
126+ s.pendingRequests.Set(permission.ID, respCh)
127+ defer s.pendingRequests.Del(permission.ID)
127128
128129 s.Publish(pubsub.CreatedEvent, permission)
129130
@@ -144,5 +145,6 @@ func NewPermissionService(workingDir string, skip bool, allowedTools []string) S
144145 sessionPermissions: make([]PermissionRequest, 0),
145146 skip: skip,
146147 allowedTools: allowedTools,
148+ pendingRequests: csync.NewMap[string, chan bool](),
147149 }
148150}
internal/tui/components/chat/sidebar/sidebar.go+31−42
@@ -4,13 +4,14 @@ import (
44 "context"
55 "fmt"
66 "os"
7+ "slices"
78 "sort"
89 "strings"
9− "sync"
1010
1111 tea "github.com/charmbracelet/bubbletea/v2"
1212 "github.com/charmbracelet/catwalk/pkg/catwalk"
1313 "github.com/charmbracelet/crush/internal/config"
14+ "github.com/charmbracelet/crush/internal/csync"
1415 "github.com/charmbracelet/crush/internal/diff"
1516 "github.com/charmbracelet/crush/internal/fsext"
1617 "github.com/charmbracelet/crush/internal/history"
@@ -71,8 +72,7 @@ type sidebarCmp struct {
7172 lspClients map[string]*lsp.Client
7273 compactMode bool
7374 history history.Service
74− // Using a sync map here because we might receive file history events concurrently
75− files sync.Map
75+ files *csync.Map[string, SessionFile]
7676}
7777
7878func New(history history.Service, lspClients map[string]*lsp.Client, compact bool) Sidebar {
@@ -80,6 +80,7 @@ func New(history history.Service, lspClients map[string]*lsp.Client, compact boo
8080 lspClients: lspClients,
8181 history: history,
8282 compactMode: compact,
83+ files: csync.NewMap[string, SessionFile](),
8384 }
8485}
8586
@@ -90,9 +91,9 @@ func (m *sidebarCmp) Init() tea.Cmd {
9091func (m *sidebarCmp) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
9192 switch msg := msg.(type) {
9293 case SessionFilesMsg:
93− m.files = sync.Map{}
94+ m.files = csync.NewMap[string, SessionFile]()
9495 for _, file := range msg.Files {
95− m.files.Store(file.FilePath, file)
96+ m.files.Set(file.FilePath, file)
9697 }
9798 return m, nil
9899
@@ -178,31 +179,29 @@ func (m *sidebarCmp) handleFileHistoryEvent(event pubsub.Event[history.File]) te
178179 return func() tea.Msg {
179180 file := event.Payload
180181 found := false
181− m.files.Range(func(key, value any) bool {
182− existing := value.(SessionFile)
183− if existing.FilePath == file.Path {
184− if existing.History.latestVersion.Version < file.Version {
185− existing.History.latestVersion = file
186− } else if file.Version == 0 {
187− existing.History.initialVersion = file
188− } else {
189− // If the version is not greater than the latest, we ignore it
190− return true
191− }
192− before := existing.History.initialVersion.Content
193− after := existing.History.latestVersion.Content
194− path := existing.History.initialVersion.Path
195− cwd := config.Get().WorkingDir()
196− path = strings.TrimPrefix(path, cwd)
197− _, additions, deletions := diff.GenerateDiff(before, after, path)
198− existing.Additions = additions
199− existing.Deletions = deletions
200− m.files.Store(file.Path, existing)
201− found = true
202− return false
182+ for existing := range m.files.Seq() {
183+ if existing.FilePath != file.Path {
184+ continue
203185 }
204− return true
205− })
186+ if existing.History.latestVersion.Version < file.Version {
187+ existing.History.latestVersion = file
188+ } else if file.Version == 0 {
189+ existing.History.initialVersion = file
190+ } else {
191+ // If the version is not greater than the latest, we ignore it
192+ continue
193+ }
194+ before := existing.History.initialVersion.Content
195+ after := existing.History.latestVersion.Content
196+ path := existing.History.initialVersion.Path
197+ cwd := config.Get().WorkingDir()
198+ path = strings.TrimPrefix(path, cwd)
199+ _, additions, deletions := diff.GenerateDiff(before, after, path)
200+ existing.Additions = additions
201+ existing.Deletions = deletions
202+ m.files.Set(file.Path, existing)
203+ found = true
204+ }
206205 if found {
207206 return nil
208207 }
@@ -215,7 +214,7 @@ func (m *sidebarCmp) handleFileHistoryEvent(event pubsub.Event[history.File]) te
215214 Additions: 0,
216215 Deletions: 0,
217216 }
218− m.files.Store(file.Path, sf)
217+ m.files.Set(file.Path, sf)
219218 return nil
220219 }
221220}
@@ -386,12 +385,7 @@ func (m *sidebarCmp) filesBlockCompact(maxWidth int) string {
386385
387386 section := t.S().Subtle.Render("Modified Files")
388387
389− files := make([]SessionFile, 0)
390− m.files.Range(func(key, value any) bool {
391− file := value.(SessionFile)
392− files = append(files, file)
393− return true
394− })
388+ files := slices.Collect(m.files.Seq())
395389
396390 if len(files) == 0 {
397391 content := lipgloss.JoinVertical(
@@ -620,12 +614,7 @@ func (m *sidebarCmp) filesBlock() string {
620614 core.Section("Modified Files", m.getMaxWidth()),
621615 )
622616
623− files := make([]SessionFile, 0)
624− m.files.Range(func(key, value any) bool {
625− file := value.(SessionFile)
626− files = append(files, file)
627− return true // continue iterating
628− })
617+ files := slices.Collect(m.files.Seq())
629618 if len(files) == 0 {
630619 return lipgloss.JoinVertical(
631620 lipgloss.Left,