W-21683590: stabilize Node 20 CI memory usage#279
Closed
Conversation
Replaces line-number-based IDs with stable qualified name format to prevent cross-file reference invalidation when files are edited. New ID format: - Uses # separator instead of : to avoid URI confusion - Dot-qualified names (e.g., file:///MyClass.cls#MyClass.myMethod) - Optional method signatures for overload disambiguation (#(String,Integer)) - No line numbers - IDs remain stable across edits Changes: - Updated UriBasedIdGenerator to generate stable IDs with parameters - Updated ApexSymbolCollectorListener to extract and pass parameter info - Updated VisibilitySymbolListener similarly - Modified SymbolFactory.generateId() to accept parameters and namespace - Updated scope path cleaning to remove implementation details (kinds, duplicates) - Fixed all test expectations to match new stable ID format Benefits: - Cross-file references remain valid when source files are edited - Method overloads have unique IDs via signature disambiguation - Simpler, more readable ID format - Backward compatible parsing of old format
… indexes
Replace DirectedGraph<ReferenceNode, ReferenceEdge> with lightweight indexes for O(1) reference lookups:
- reverseIndex: Map<targetSymbolId, Set<refKey>> for findReferencesTo
- forwardIndex: Map<sourceFileUri, Set<refKey>> for file cleanup
- refStore: Map<refKey, RefStoreEntry> for full reference details
- refKey format: {sourceFileUri}:{sourceSymbolId}:{refIndex}
Benefits:
- O(1) lookups instead of O(S×N) graph scans
- Built from SymbolTable.getAllReferences() (single source of truth)
- Duplicate reference detection
- Maintains backward compatibility for visualization
Updated methods in ApexSymbolGraph: addSymbol, addReferenceToGraph, findReferencesTo, findReferencesFrom, removeFile, detectCircularDependencies, and all deferred reference processing methods.
Updated extractGraphData.ts to use new indexes for graph visualization.
All 2,682 tests passing.
…traction
Extract repeated code patterns into reusable helper methods:
1. ApexSymbolGraph.ts:
- createAndAddReference(): Consolidates 5 duplicate blocks (~100 lines)
that created RefStoreEntry and added to indexes
- Used in: addReferenceToGraph, all deferred reference processing methods
2. extractGraphData.ts:
- createGraphNode(): Consolidates 3 duplicate blocks (~60 lines)
that calculated reference counts and built GraphNode structures
- Used in: getAllNodesEffect, getGraphDataForFile, getGraphDataByType
Total reduction: 167 lines removed, 113 lines added (net -54 lines)
All 2,682 tests passing.
…eyword - Define inTypeSymbolGroup in symbol.ts; re-export from symbolNarrowing; FQNUtils.isType delegates\n- Replace duplicate type-kind checks with inTypeSymbolGroup across listeners and ApexSymbolManager\n- Extract applyModifierKeyword shared by Visibility and ApexSymbol collectors\n- Add jscpd (check:dupes, parser-ast scoped scripts); track .jscpd.json; verification skill Made-with: Cursor
…GSEGV Parallel workers loading protobuf/stdlib could segfault; recycle workers at 512MB idle heap. Made-with: Cursor
Made-with: Cursor
…andling W-21638371 Replace UriProtocol union, getProtocolType, hasProtocol, isUserCodeUri, and PROTOCOL_PREFIXES with a single hasUriScheme boolean check. Only apexlib:// is special-cased (in getFilePathFromUri); file://, memfs:, and all other schemes are pass-through. Remove dead exports: createBuiltinUri, extractBuiltinType, isBuiltinUri, convertToAppropriateUri, BUILTIN_URI_PREFIX, isUserCodeId. Made-with: Cursor
…lManager createFileUri already checks for existing schemes internally, so the hasUriScheme(x) ? x : createFileUri(x) pattern was redundant. Made-with: Cursor
…naming - Use apexlib:// for void/null synthetic symbols; graph virtual IDs via generateSymbolId - Rename resolveBuiltInType, findBuiltInType, FQNUtils helpers; add isStandardLibraryTypeInfo - Deprecate proto is_built_in naming in comments; ConfigurationSummary defaultDocumentSchemes - Cap Jest maxWorkers to reduce worker SIGSEGV flakiness Made-with: Cursor
@vscode/test-web only selects the stable web build when quality is the literal 'stable'. Passing version 1.108.0 downloaded latest Insider instead. Use quality: stable for semver pins from readLocalVSCodeVersion(). Made-with: Cursor
…p-compliant-services 4 workers × ~2 GB stdlib each saturates the 16 GB CI runner, causing OOM crashes in HoverProcessingService.integration.test.ts and ApexSymbolManager.resolution.test.ts. Changes: - apex-parser-ast: reduce maxWorkers from 4 to 2 - lsp-compliant-services: add workerIdleMemoryLimit 512MB, maxWorkers 2, testTimeout 120s Made-with: Cursor
Node 20.x default V8 heap cap (~4 GB) is too low for heavy stdlib-loading test suites (HoverProcessingService, ApexSymbolManager.resolution). The 16 GB CI runners have capacity but the per-process limit triggers OOM before memory is actually exhausted. Made-with: Cursor
kylewalke
changed the title
With maxWorkers:2, both workers stay continuously busy — the worker never becomes idle between suites so workerIdleMemoryLimit never fires. Memory accumulates across all suites in one long-lived worker and OOMs at ~8 GB on Node 20.x. With maxWorkers:1, the single worker briefly becomes idle between suites, letting workerIdleMemoryLimit recycle it at 512 MB. Each fresh worker only holds stdlib + one suite at a time (~2 GB peak vs ~8 GB accumulated). Revert the NODE_OPTIONS --max-old-space-size change (higher limit defers GC but doesn't reduce total retention; the real fix is worker recycling). Made-with: Cursor
E2E Test Results SummaryRun: 273 Test Results
Passing Rate by File
ArtifactsView detailed test reports in the Artifacts section. All tests passed |
Node 20.x consistently OOMs in coverage runs for heavy stdlib suites, while lts/* and node matrix legs pass. Run `npm test` on 20.x and keep `test:coverage` for the other matrix versions so CI remains stable while still collecting coverage artifacts. Made-with: Cursor
E2E Test Results SummaryRun: 275 Test Results
Passing Rate by File
ArtifactsView detailed test reports in the Artifacts section. All tests passed |
After switching 20.x to `npm test`, lsp-compliant-services still OOMs at Node 20's ~4 GB default old-space limit. Set NODE_OPTIONS=--max-old-space-size=6144 only for the 20.x non-coverage step to keep memory below the prior 8 GB regression while unblocking heavy stdlib test suites. Made-with: Cursor
kylewalke
changed the title
E2E Test Results SummaryRun: 278 Test Results
Passing Rate by File
ArtifactsView detailed test reports in the Artifacts section. All tests passed |
The scoped 6 GB heap bump still OOMs in HoverProcessingService on both ubuntu and windows. Increase the Node 20 non-coverage step to 8 GB so the heavy stdlib integration suites can finish, while keeping coverage disabled only on 20.x. Made-with: Cursor
E2E Test Results SummaryRun: 279 Test Results
Passing Rate by File
ArtifactsView detailed test reports in the Artifacts section. All tests passed |
E2E Test Results SummaryRun: 284 Test Results
Passing Rate by File
ArtifactsView detailed test reports in the Artifacts section. All tests passed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
npm testinstead ofnpm run test:coverageon the20.xmatrix leg to avoid the coverage-specific memory blowupNODE_OPTIONS=--max-old-space-size=8192only for the Node 20 non-coverage step so heavy stdlib suites can complete under the lower Node 20 heap ceilinglts/*andnodematrix legs where those jobs already run reliablyWork Item
@W-21683590@
Test plan
Test (ubuntu-latest, 20.x)passesTest (windows-latest, 20.x)passes