Skip to content

Commit 8c000cb

Browse files
authored
🐛 perf: Optimize NodeJS provider file processing (fixes #969) (#970)
## Summary Eliminates dual bottlenecks in TypeScript/JavaScript file analysis that caused 20-25 minute delays on large codebases. Fixes #969 ## Problem The NodeJS provider had **two major bottlenecks**: 1. **Sleep bottleneck**: 2-second sleep before EACH file - 582 files × 2s = 1,164 seconds (~19 minutes) of pure waiting 2. **Query bottleneck**: `GetAllDeclarations` called 18 times - Once per 32-file batch instead of once total - Each call queries the entire workspace for symbols (expensive operation) ## Solution Modified `external-providers/generic-external-provider/pkg/server_configurations/nodejs/service_client.go`: **Before:** - Batch processing (32 files at a time) - 2-second sleep before each `didOpen` notification - `GetAllDeclarations` called after each batch (18 times total) **After:** - Open all files in single loop (no per-file sleep) - Single 500ms sleep after all `didOpen` notifications sent - Call `GetAllDeclarations` once after all files are indexed - Simplified `didClose` loop The single sleep after all files prevents the race condition (symbol queries happening before LSP server finishes indexing) without the massive overhead of sleeping before each file. ## Performance Results **Test environment:** tackle2-ui (582 TypeScript files, 56 PatternFly rules) | Metric | Before | After | Improvement | |--------|--------|-------|-------------| | **Total time** | 20-25 minutes | **1m 43s** | **91% faster** | | **Actual time** | 1200-1500s | **103 seconds** | **12x speedup** | | **Sleep overhead** | 1,164s | 0.5s | 99.96% reduction | | **Workspace queries** | 18 calls | 1 call | 94% reduction | ## Testing - ✅ Output file generated successfully (213 KB, 2,982 lines) - ✅ Violations detected correctly (PatternFly migration issues) - ✅ No errors or warnings in execution - ✅ Tested on both small and large TypeScript codebases ## Additional Notes This fix is even better than the original proposal because it eliminates both the sleep bottleneck AND the redundant workspace queries. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Optimized file processing flow for improved performance and reliability by consolidating file operations into a streamlined single-pass approach with coordinated resource management. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: tsanders <tsanders@redhat.com>
1 parent 5d44057 commit 8c000cb

1 file changed

Lines changed: 22 additions & 27 deletions

File tree

  • external-providers/generic-external-provider/pkg/server_configurations/nodejs

external-providers/generic-external-provider/pkg/server_configurations/nodejs/service_client.go

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -200,44 +200,39 @@ func (sc *NodeServiceClient) EvaluateReferenced(ctx context.Context, cap string,
200200
return sc.Conn.Notify(ctx, "textDocument/didClose", params)
201201
}
202202

203-
BATCH_SIZE := 32
204-
batchRight, batchLeft := 0, 0
205-
var symbols []protocol.WorkspaceSymbol
206-
for batchRight < len(nodeFiles) {
207-
for batchRight-batchLeft < BATCH_SIZE && batchRight < len(nodeFiles) {
208-
fileInfo := nodeFiles[batchRight]
209-
trimmedURI := strings.TrimPrefix(fileInfo.path, "file://")
210-
text, err := os.ReadFile(trimmedURI)
211-
if err != nil {
212-
return provider.ProviderEvaluateResponse{}, err
213-
}
214-
215-
// TODO eemcmullan: look into better solution
216-
// didOpen calls conn.Notify, which does not wait for a response
217-
// for small apps, this can cause another conn.Notify call before the previous completes
218-
// resulting in missing incidents
219-
time.Sleep(2 * time.Second)
220-
err = didOpen(fileInfo.path, fileInfo.langID, text)
221-
if err != nil {
222-
return provider.ProviderEvaluateResponse{}, err
223-
}
203+
// Open all files first
204+
for _, fileInfo := range nodeFiles {
205+
trimmedURI := strings.TrimPrefix(fileInfo.path, "file://")
206+
text, err := os.ReadFile(trimmedURI)
207+
if err != nil {
208+
return provider.ProviderEvaluateResponse{}, err
209+
}
224210

225-
batchRight++
211+
// didOpen calls conn.Notify, which does not wait for a response
212+
err = didOpen(fileInfo.path, fileInfo.langID, text)
213+
if err != nil {
214+
return provider.ProviderEvaluateResponse{}, err
226215
}
227-
symbols = sc.GetAllDeclarations(ctx, sc.BaseConfig.WorkspaceFolders, query)
228-
batchLeft = batchRight
229216
}
230217

218+
// Sleep once after all files are opened to allow LSP server to process
219+
// all didOpen notifications before querying for symbols.
220+
// This prevents the race condition without requiring sleep before each file.
221+
time.Sleep(500 * time.Millisecond)
222+
223+
// Query symbols once after all files are indexed
224+
symbols := sc.GetAllDeclarations(ctx, sc.BaseConfig.WorkspaceFolders, query)
225+
231226
incidentsMap, err := sc.EvaluateSymbols(ctx, symbols)
232227
if err != nil {
233228
return resp{}, err
234229
}
235230

236-
for batchLeft < batchRight {
237-
if err = didClose(nodeFiles[batchLeft].path); err != nil {
231+
// Close all opened files
232+
for _, fileInfo := range nodeFiles {
233+
if err = didClose(fileInfo.path); err != nil {
238234
return provider.ProviderEvaluateResponse{}, err
239235
}
240-
batchLeft++
241236
}
242237

243238
incidents := []provider.IncidentContext{}

0 commit comments

Comments
 (0)