on main help sign in
perf: remove mutex from lsp manager
eecb1bfCarlos Alexandro Becker
internal/agent/tools/glob.go+1−1
@@ -81,7 +81,7 @@ func globFiles(ctx context.Context, pattern, searchPath string, limit int) ([]st
81 slog.Warn("Ripgrep execution failed, falling back to doublestar", "error", err)
81 slog.Warn("Ripgrep execution failed, falling back to doublestar", "error", err)
82 }
82 }
83
83
84− return fsext.GlobWithDoubleStar(pattern, searchPath, limit)
84+ return fsext.GlobGitignoreAware(pattern, searchPath, limit)
85}
85}
86
86
87func runRipgrep(cmd *exec.Cmd, searchRoot string, limit int) ([]string, error) {
87func runRipgrep(cmd *exec.Cmd, searchRoot string, limit int) ([]string, error) {
internal/fsext/fileutil.go+15−3
@@ -82,7 +82,19 @@ func (w *FastGlobWalker) ShouldSkipDir(path string) bool {
82 return w.directoryLister.shouldIgnore(path, nil, true)
82 return w.directoryLister.shouldIgnore(path, nil, true)
83}
83}
84
84
85+// Glob globs files.
86+//
87+// Does not respect gitignore.
88+func Glob(pattern string, cwd string, limit int) ([]string, bool, error) {
89+ return globWithDoubleStar(pattern, cwd, limit, false)
90+}
91+
92+// GlobGitignoreAware globs files respecting gitignore.
93+func GlobGitignoreAware(pattern string, cwd string, limit int) ([]string, bool, error) {
94+ return globWithDoubleStar(pattern, cwd, limit, true)
95+}
96+
85−func GlobWithDoubleStar(pattern, searchPath string, limit int) ([]string, bool, error) {
97+func globWithDoubleStar(pattern, searchPath string, limit int, gitignore bool) ([]string, bool, error) {
86 // Normalize pattern to forward slashes on Windows so their config can use
98 // Normalize pattern to forward slashes on Windows so their config can use
87 // backslashes
99 // backslashes
88 pattern = filepath.ToSlash(pattern)
100 pattern = filepath.ToSlash(pattern)
@@ -101,11 +113,11 @@ func GlobWithDoubleStar(pattern, searchPath string, limit int) ([]string, bool,
101
113
102 isDir := d.IsDir()
114 isDir := d.IsDir()
103 if isDir {
115 if isDir {
104− if walker.ShouldSkipDir(path) {
116+ if gitignore && walker.ShouldSkipDir(path) {
105 return filepath.SkipDir
117 return filepath.SkipDir
106 }
118 }
107 } else {
119 } else {
108− if walker.ShouldSkip(path) {
120+ if gitignore && walker.ShouldSkip(path) {
109 return nil
121 return nil
110 }
122 }
111 }
123 }
internal/fsext/fileutil_test.go+13−13
@@ -24,7 +24,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
24 require.NoError(t, os.WriteFile(file, []byte("test content"), 0o644))
24 require.NoError(t, os.WriteFile(file, []byte("test content"), 0o644))
25 }
25 }
26
26
27− matches, truncated, err := GlobWithDoubleStar("**/main.go", testDir, 0)
27+ matches, truncated, err := GlobGitignoreAware("**/main.go", testDir, 0)
28 require.NoError(t, err)
28 require.NoError(t, err)
29 require.False(t, truncated)
29 require.False(t, truncated)
30
30
@@ -47,7 +47,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
47 require.NoError(t, os.WriteFile(filepath.Join(srcDir, "main.go"), []byte("package main"), 0o644))
47 require.NoError(t, os.WriteFile(filepath.Join(srcDir, "main.go"), []byte("package main"), 0o644))
48 require.NoError(t, os.WriteFile(pkgFile, []byte("test"), 0o644))
48 require.NoError(t, os.WriteFile(pkgFile, []byte("test"), 0o644))
49
49
50− matches, truncated, err := GlobWithDoubleStar("pkg", testDir, 0)
50+ matches, truncated, err := GlobGitignoreAware("pkg", testDir, 0)
51 require.NoError(t, err)
51 require.NoError(t, err)
52 require.False(t, truncated)
52 require.False(t, truncated)
53
53
@@ -66,7 +66,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
66 require.NoError(t, os.MkdirAll(dir, 0o755))
66 require.NoError(t, os.MkdirAll(dir, 0o755))
67 }
67 }
68
68
69− matches, truncated, err := GlobWithDoubleStar("**/pkg", testDir, 0)
69+ matches, truncated, err := GlobGitignoreAware("**/pkg", testDir, 0)
70 require.NoError(t, err)
70 require.NoError(t, err)
71 require.False(t, truncated)
71 require.False(t, truncated)
72
72
@@ -95,7 +95,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
95 require.NoError(t, os.WriteFile(file, []byte("package main"), 0o644))
95 require.NoError(t, os.WriteFile(file, []byte("package main"), 0o644))
96 }
96 }
97
97
98− matches, truncated, err := GlobWithDoubleStar("pkg/**", testDir, 0)
98+ matches, truncated, err := GlobGitignoreAware("pkg/**", testDir, 0)
99 require.NoError(t, err)
99 require.NoError(t, err)
100 require.False(t, truncated)
100 require.False(t, truncated)
101
101
@@ -124,7 +124,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
124 require.NoError(t, os.WriteFile(file, []byte("test"), 0o644))
124 require.NoError(t, os.WriteFile(file, []byte("test"), 0o644))
125 }
125 }
126
126
127− matches, truncated, err := GlobWithDoubleStar("**/*.txt", testDir, 5)
127+ matches, truncated, err := GlobGitignoreAware("**/*.txt", testDir, 5)
128 require.NoError(t, err)
128 require.NoError(t, err)
129 require.True(t, truncated, "Expected truncation with limit")
129 require.True(t, truncated, "Expected truncation with limit")
130 require.Len(t, matches, 5, "Expected exactly 5 matches with limit")
130 require.Len(t, matches, 5, "Expected exactly 5 matches with limit")
@@ -143,7 +143,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
143 require.NoError(t, os.WriteFile(file, []byte("test"), 0o644))
143 require.NoError(t, os.WriteFile(file, []byte("test"), 0o644))
144 }
144 }
145
145
146− matches, truncated, err := GlobWithDoubleStar("a/b/c/file1.txt", testDir, 0)
146+ matches, truncated, err := GlobGitignoreAware("a/b/c/file1.txt", testDir, 0)
147 require.NoError(t, err)
147 require.NoError(t, err)
148 require.False(t, truncated)
148 require.False(t, truncated)
149
149
@@ -171,7 +171,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
171 require.NoError(t, os.Chtimes(file2, m2, m2))
171 require.NoError(t, os.Chtimes(file2, m2, m2))
172 require.NoError(t, os.Chtimes(file3, m3, m3))
172 require.NoError(t, os.Chtimes(file3, m3, m3))
173
173
174− matches, truncated, err := GlobWithDoubleStar("*.txt", testDir, 0)
174+ matches, truncated, err := GlobGitignoreAware("*.txt", testDir, 0)
175 require.NoError(t, err)
175 require.NoError(t, err)
176 require.False(t, truncated)
176 require.False(t, truncated)
177
177
@@ -181,7 +181,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
181 t.Run("handles empty directory", func(t *testing.T) {
181 t.Run("handles empty directory", func(t *testing.T) {
182 testDir := t.TempDir()
182 testDir := t.TempDir()
183
183
184− matches, truncated, err := GlobWithDoubleStar("**", testDir, 0)
184+ matches, truncated, err := GlobGitignoreAware("**", testDir, 0)
185 require.NoError(t, err)
185 require.NoError(t, err)
186 require.False(t, truncated)
186 require.False(t, truncated)
187 // Even empty directories should return the directory itself
187 // Even empty directories should return the directory itself
@@ -191,7 +191,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
191 t.Run("handles non-existent search path", func(t *testing.T) {
191 t.Run("handles non-existent search path", func(t *testing.T) {
192 nonExistentDir := filepath.Join(t.TempDir(), "does", "not", "exist")
192 nonExistentDir := filepath.Join(t.TempDir(), "does", "not", "exist")
193
193
194− matches, truncated, err := GlobWithDoubleStar("**", nonExistentDir, 0)
194+ matches, truncated, err := GlobGitignoreAware("**", nonExistentDir, 0)
195 require.Error(t, err, "Should return error for non-existent search path")
195 require.Error(t, err, "Should return error for non-existent search path")
196 require.False(t, truncated)
196 require.False(t, truncated)
197 require.Empty(t, matches)
197 require.Empty(t, matches)
@@ -219,17 +219,17 @@ func TestGlobWithDoubleStar(t *testing.T) {
219 ignoredFileInDir := filepath.Join(testDir, "backup", "old.txt")
219 ignoredFileInDir := filepath.Join(testDir, "backup", "old.txt")
220 require.NoError(t, os.WriteFile(ignoredFileInDir, []byte("old content"), 0o644))
220 require.NoError(t, os.WriteFile(ignoredFileInDir, []byte("old content"), 0o644))
221
221
222− matches, truncated, err := GlobWithDoubleStar("*.tmp", testDir, 0)
222+ matches, truncated, err := GlobGitignoreAware("*.tmp", testDir, 0)
223 require.NoError(t, err)
223 require.NoError(t, err)
224 require.False(t, truncated)
224 require.False(t, truncated)
225 require.Empty(t, matches, "Expected no matches for '*.tmp' pattern (should be ignored)")
225 require.Empty(t, matches, "Expected no matches for '*.tmp' pattern (should be ignored)")
226
226
227− matches, truncated, err = GlobWithDoubleStar("backup", testDir, 0)
227+ matches, truncated, err = GlobGitignoreAware("backup", testDir, 0)
228 require.NoError(t, err)
228 require.NoError(t, err)
229 require.False(t, truncated)
229 require.False(t, truncated)
230 require.Empty(t, matches, "Expected no matches for 'backup' pattern (should be ignored)")
230 require.Empty(t, matches, "Expected no matches for 'backup' pattern (should be ignored)")
231
231
232− matches, truncated, err = GlobWithDoubleStar("*.txt", testDir, 0)
232+ matches, truncated, err = GlobGitignoreAware("*.txt", testDir, 0)
233 require.NoError(t, err)
233 require.NoError(t, err)
234 require.False(t, truncated)
234 require.False(t, truncated)
235 require.Equal(t, []string{goodFile}, matches)
235 require.Equal(t, []string{goodFile}, matches)
@@ -257,7 +257,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
257 require.NoError(t, os.Chtimes(middleDir, tMiddle, tMiddle))
257 require.NoError(t, os.Chtimes(middleDir, tMiddle, tMiddle))
258 require.NoError(t, os.Chtimes(oldestFile, tNewest, tNewest))
258 require.NoError(t, os.Chtimes(oldestFile, tNewest, tNewest))
259
259
260− matches, truncated, err := GlobWithDoubleStar("*.rs", testDir, 0)
260+ matches, truncated, err := GlobGitignoreAware("*.rs", testDir, 0)
261 require.NoError(t, err)
261 require.NoError(t, err)
262 require.False(t, truncated)
262 require.False(t, truncated)
263 require.Len(t, matches, 3)
263 require.Len(t, matches, 3)
internal/lsp/client.go+1−1
@@ -85,7 +85,7 @@ func New(
85 resolver: resolver,
85 resolver: resolver,
86 cwd: cwd,
86 cwd: cwd,
87 }
87 }
88− client.serverState.Store(StateStarting)
88+ client.serverState.Store(StateStopped)
89
89
90 if err := client.createPowernapClient(); err != nil {
90 if err := client.createPowernapClient(); err != nil {
91 return nil, err
91 return nil, err
internal/lsp/manager.go+52−40
@@ -21,13 +21,14 @@ import (
21 "github.com/sourcegraph/jsonrpc2"
21 "github.com/sourcegraph/jsonrpc2"
22)
22)
23
23
24+var unavailable = csync.NewMap[string, struct{}]()
25+
24// Manager handles lazy initialization of LSP clients based on file types.
26// Manager handles lazy initialization of LSP clients based on file types.
25type Manager struct {
27type Manager struct {
26 clients *csync.Map[string, *Client]
28 clients *csync.Map[string, *Client]
27 cfg *config.Config
29 cfg *config.Config
28 manager *powernapconfig.Manager
30 manager *powernapconfig.Manager
29 callback func(name string, client *Client)
31 callback func(name string, client *Client)
30− mu sync.Mutex
31}
32}
32
33
33// NewManager creates a new LSP manager service.
34// NewManager creates a new LSP manager service.
@@ -72,17 +73,12 @@ func (s *Manager) Clients() *csync.Map[string, *Client] {
72// SetCallback sets a callback that is invoked when a new LSP
73// SetCallback sets a callback that is invoked when a new LSP
73// client is successfully started. This allows the coordinator to add LSP tools.
74// client is successfully started. This allows the coordinator to add LSP tools.
74func (s *Manager) SetCallback(cb func(name string, client *Client)) {
75func (s *Manager) SetCallback(cb func(name string, client *Client)) {
75− s.mu.Lock()
76− defer s.mu.Unlock()
77 s.callback = cb
76 s.callback = cb
78}
77}
79
78
80// TrackConfigured will callback the user-configured LSPs, but will not create
79// TrackConfigured will callback the user-configured LSPs, but will not create
81// any clients.
80// any clients.
82func (s *Manager) TrackConfigured() {
81func (s *Manager) TrackConfigured() {
83− s.mu.Lock()
84− defer s.mu.Unlock()
85−
86 var wg sync.WaitGroup
82 var wg sync.WaitGroup
87 for name := range s.manager.GetServers() {
83 for name := range s.manager.GetServers() {
88 if !s.isUserConfigured(name) {
84 if !s.isUserConfigured(name) {
@@ -102,16 +98,10 @@ func (s *Manager) Start(ctx context.Context, path string) {
102 return
98 return
103 }
99 }
104
100
105− s.mu.Lock()
106− defer s.mu.Unlock()
107−
108 var wg sync.WaitGroup
101 var wg sync.WaitGroup
109 for name, server := range s.manager.GetServers() {
102 for name, server := range s.manager.GetServers() {
110− if !handles(server, path, s.cfg.WorkingDir()) {
111− continue
112− }
113 wg.Go(func() {
103 wg.Go(func() {
114− s.startServer(ctx, name, server)
104+ s.startServer(ctx, name, path, server)
115 })
105 })
116 }
106 }
117 wg.Wait()
107 wg.Wait()
@@ -149,12 +139,31 @@ var skipAutoStartCommands = map[string]bool{
149 "tflint": true,
139 "tflint": true,
150}
140}
151
141
152−func (s *Manager) startServer(ctx context.Context, name string, server *powernapconfig.ServerConfig) {
142+func (s *Manager) startServer(ctx context.Context, name, filepath string, server *powernapconfig.ServerConfig) {
143+ cfg := s.buildConfig(name, server)
144+ if cfg.Disabled {
145+ return
146+ }
147+
148+ if _, exists := unavailable.Get(name); exists {
149+ return
150+ }
151+
152+ if client, ok := s.clients.Get(name); ok {
153+ switch client.GetServerState() {
154+ case StateReady, StateStarting, StateDisabled:
155+ s.callback(name, client)
156+ // already done, return
157+ return
158+ }
159+ }
160+
153 userConfigured := s.isUserConfigured(name)
161 userConfigured := s.isUserConfigured(name)
154
162
155 if !userConfigured {
163 if !userConfigured {
156 if _, err := exec.LookPath(server.Command); err != nil {
164 if _, err := exec.LookPath(server.Command); err != nil {
157 slog.Debug("LSP server not installed, skipping", "name", name, "command", server.Command)
165 slog.Debug("LSP server not installed, skipping", "name", name, "command", server.Command)
166+ unavailable.Set(name, struct{}{})
158 return
167 return
159 }
168 }
160 if skipAutoStartCommands[server.Command] {
169 if skipAutoStartCommands[server.Command] {
@@ -163,34 +172,43 @@ func (s *Manager) startServer(ctx context.Context, name string, server *powernap
163 }
172 }
164 }
173 }
165
174
166− cfg := s.buildConfig(name, server)
175+ // this is the slowest bit, so we do it last.
167− if client, ok := s.clients.Get(name); ok {
176+ if !handles(server, filepath, s.cfg.WorkingDir()) {
168− switch client.GetServerState() {
177+ // nothing to do
169− case StateReady, StateStarting:
178+ return
170− s.callback(name, client)
171− // already done, return
172− return
173− }
174 }
179 }
175− client, err := New(
180+
181+ // check again in case another goroutine started it in the meantime
182+ var err error
183+ client := s.clients.GetOrSet(name, func() *Client {
184+ var cli *Client
185+ cli, err = New(
176− ctx,
186+ ctx,
177− name,
187+ name,
178− cfg,
188+ cfg,
179− s.cfg.Resolver(),
189+ s.cfg.Resolver(),
180− s.cfg.WorkingDir(),
190+ s.cfg.WorkingDir(),
181− s.cfg.Options.DebugLSP,
191+ s.cfg.Options.DebugLSP,
192+ )
193+ return cli
182− )
194+ })
183 if err != nil {
195 if err != nil {
184 slog.Error("Failed to create LSP client", "name", name, "error", err)
196 slog.Error("Failed to create LSP client", "name", name, "error", err)
185 return
197 return
186 }
198 }
187− s.callback(name, client)
188−
189 defer func() {
199 defer func() {
190− s.clients.Set(name, client)
191 s.callback(name, client)
200 s.callback(name, client)
192 }()
201 }()
193
202
203+ switch client.GetServerState() {
204+ case StateReady, StateStarting:
205+ // already done, return
206+ return
207+ }
208+
209+ client.serverState.Store(StateStarting)
210+ s.callback(name, client)
211+
194 initCtx, cancel := context.WithTimeout(ctx, time.Duration(cmp.Or(cfg.Timeout, 30))*time.Second)
212 initCtx, cancel := context.WithTimeout(ctx, time.Duration(cmp.Or(cfg.Timeout, 30))*time.Second)
195 defer cancel()
213 defer cancel()
196
214
@@ -271,7 +289,7 @@ func hasRootMarkers(dir string, markers []string) bool {
271 }
289 }
272 for _, pattern := range markers {
290 for _, pattern := range markers {
273 // Use fsext.GlobWithDoubleStar to find matches
291 // Use fsext.GlobWithDoubleStar to find matches
274− matches, _, err := fsext.GlobWithDoubleStar(pattern, dir, 1)
292+ matches, _, err := fsext.Glob(pattern, dir, 1)
275 if err == nil && len(matches) > 0 {
293 if err == nil && len(matches) > 0 {
276 return true
294 return true
277 }
295 }
@@ -291,9 +309,6 @@ func handles(server *powernapconfig.ServerConfig, filePath, workDir string) bool
291// in the middle of writing something.
309// in the middle of writing something.
292// Generally it doesn't matter when shutting down Crush, though.
310// Generally it doesn't matter when shutting down Crush, though.
293func (s *Manager) KillAll(context.Context) {
311func (s *Manager) KillAll(context.Context) {
294− s.mu.Lock()
295− defer s.mu.Unlock()
296−
297 var wg sync.WaitGroup
312 var wg sync.WaitGroup
298 for name, client := range s.clients.Seq2() {
313 for name, client := range s.clients.Seq2() {
299 wg.Go(func() {
314 wg.Go(func() {
@@ -308,9 +323,6 @@ func (s *Manager) KillAll(context.Context) {
308
323
309// StopAll stops all running LSP clients and clears the client map.
324// StopAll stops all running LSP clients and clears the client map.
310func (s *Manager) StopAll(ctx context.Context) {
325func (s *Manager) StopAll(ctx context.Context) {
311− s.mu.Lock()
312− defer s.mu.Unlock()
313−
314 var wg sync.WaitGroup
326 var wg sync.WaitGroup
315 for name, client := range s.clients.Seq2() {
327 for name, client := range s.clients.Seq2() {
316 wg.Go(func() {
328 wg.Go(func() {
internal/ui/model/lsp.go+1−1
@@ -108,7 +108,7 @@ func lspList(t *styles.Styles, lsps []LSPInfo, width, maxItems int) string {
108 icon = t.ResourceOfflineIcon.Foreground(t.Muted.GetBackground()).String()
108 icon = t.ResourceOfflineIcon.Foreground(t.Muted.GetBackground()).String()
109 description = t.ResourceStatus.Render("disabled")
109 description = t.ResourceStatus.Render("disabled")
110 default:
110 default:
111− icon = t.ResourceOfflineIcon.String()
111+ continue
112 }
112 }
113 renderedLsps = append(renderedLsps, common.Status(t, common.StatusOpts{
113 renderedLsps = append(renderedLsps, common.Status(t, common.StatusOpts{
114 Icon: icon,
114 Icon: icon,