From 70dfd618108197337affc30a75e3224de7c6ce96 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Mon, 11 May 2026 15:34:23 -0300 Subject: [PATCH 1/3] Reduce memory retention in caches and dispose paths --- src/Scope.spec.ts | 10 ++++++++++ src/Scope.ts | 2 ++ src/files/BrsFile.spec.ts | 12 ++++++++++++ src/files/BrsFile.ts | 1 + src/util.spec.ts | 15 +++++++++++++++ src/util.ts | 13 +++++++++++-- 6 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/Scope.spec.ts b/src/Scope.spec.ts index a5c93d16f..ef3ca6a74 100644 --- a/src/Scope.spec.ts +++ b/src/Scope.spec.ts @@ -205,6 +205,16 @@ describe('Scope', () => { expect(scopes.length).to.equal(2); }); + it('clears cache when disposed', () => { + const scope = program.getScopeByName('source'); + scope.getAllFiles(); + expect(scope['cache'].size).to.be.greaterThan(0); + + scope.dispose(); + + expect(scope['cache'].size).to.equal(0); + }); + describe('addFile', () => { it('detects callables from all loaded files', () => { const sourceScope = program.getScopeByName('source'); diff --git a/src/Scope.ts b/src/Scope.ts index 40be3b5a7..576d7f7ac 100644 --- a/src/Scope.ts +++ b/src/Scope.ts @@ -528,6 +528,8 @@ export class Scope { * Clean up all event handles */ public dispose() { + this.unlinkSymbolTable(); + this.cache.clear(); this.unsubscribeFromDependencyGraph?.(); } diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 3398220c3..5baee8896 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -103,6 +103,18 @@ describe('BrsFile', () => { program.validate(); expectZeroDiagnostics(program); }); + + it('clears cached lookups', () => { + const file = program.setFile('source/main.bs', `sub main(): end sub`); + // prime lookup cache + void file['_cachedLookups'].functionStatements; + const cache = file['_cachedLookups']['cache']; + expect(cache.size).to.be.greaterThan(0); + + file.dispose(); + + expect(cache.size).to.equal(0); + }); }); describe('allowBrighterScriptInBrightScript', () => { diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index 2190a60e7..c60a64aa4 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -1386,6 +1386,7 @@ export class BrsFile implements BscFile { } public dispose() { + this._cachedLookups?.invalidate(); this._parser?.dispose(); //deleting these properties result in lower memory usage (garbage collection is magic!) diff --git a/src/util.spec.ts b/src/util.spec.ts index 9ddf55cb3..40a535f43 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -1659,6 +1659,7 @@ describe('util', () => { describe('standardizePath', () => { let isWindowsOrig = util['isWindows']; let isWindows = isWindowsOrig; + let standardizePathCacheLimitOrig = util['standardizePathCacheLimit']; beforeEach(() => { util['standardizePathCache'].clear(); @@ -1666,6 +1667,7 @@ describe('util', () => { afterEach(() => { util['standardizePathCache'].clear(); util['isWindows'] = isWindowsOrig; + util['standardizePathCacheLimit'] = standardizePathCacheLimitOrig; }); function test(incoming: string, expected: string) { @@ -1704,6 +1706,19 @@ describe('util', () => { }); }); + it('bounds cache growth by clearing when limit is reached', () => { + util['standardizePathCacheLimit'] = 2; + + util.standardizePath('source/a.bs'); + util.standardizePath('source/b.bs'); + expect(util['standardizePathCache'].size).to.equal(2); + + util.standardizePath('source/c.bs'); + + expect(util['standardizePathCache'].size).to.equal(1); + expect(util['standardizePathCache'].has('source/c.bs')).to.be.true; + }); + describe('windows paths on unix', () => { beforeEach(() => { isWindows = false; diff --git a/src/util.ts b/src/util.ts index e9baae08c..459b3e80c 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1836,6 +1836,8 @@ export class Util { private isWindows = process.platform === 'win32'; private standardizePathCache = new Map(); + // Prevent unbounded growth in long-lived language-server sessions. + private standardizePathCacheLimit = 20_000; /** * Converts a path into a standardized format (drive letter to lower, remove extra slashes, use single slash type, resolve relative parts, etc...) @@ -1855,7 +1857,7 @@ export class Util { if (/^virtual:[\/\\]/i.test(thePath)) { // Strip the `virtual:` prefix, normalize slashes, then re-add the prefix thePath = 'virtual:/' + thePath.slice(8).replace(/^[\/\\]+/, '').replace(/[\/\\]+/g, '/').toLowerCase(); - this.standardizePathCache.set(originalPath, thePath); + this.addToStandardizePathCache(originalPath, thePath); return thePath; } @@ -1880,10 +1882,17 @@ export class Util { thePath = String.fromCharCode(firstChar + 32) + thePath.slice(1); } } - this.standardizePathCache.set(originalPath, thePath); + this.addToStandardizePathCache(originalPath, thePath); return thePath; } + private addToStandardizePathCache(key: string, value: string) { + if (this.standardizePathCache.size >= this.standardizePathCacheLimit) { + this.standardizePathCache.clear(); + } + this.standardizePathCache.set(key, value); + } + /** * Given a Diagnostic or BsDiagnostic, return a deep clone of the diagnostic. * @param diagnostic the diagnostic to clone From 53eb5af3d2be097d99604afd41afe91138432237 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Tue, 12 May 2026 16:14:59 -0300 Subject: [PATCH 2/3] Only create duplicates map and leading trivia array when needed --- src/CrossScopeValidator.ts | 9 ++++-- .../completions/CompletionsProcessor.ts | 6 ++-- src/files/BrsFile.spec.ts | 2 +- src/lexer/Lexer.spec.ts | 28 +++++++++++++++++-- src/lexer/Lexer.ts | 8 +++--- src/lexer/Token.ts | 2 +- src/parser/Statement.ts | 6 ++-- src/parser/TranspileState.ts | 8 +++++- src/util.ts | 15 +++++++--- 9 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/CrossScopeValidator.ts b/src/CrossScopeValidator.ts index 4aaf0efb2..2007f1c71 100644 --- a/src/CrossScopeValidator.ts +++ b/src/CrossScopeValidator.ts @@ -278,7 +278,7 @@ export class CrossScopeValidator { return this.providedTreeMap.get(scope.name); } const providedTree = new ProvidedNode('', this.componentsMap); - const duplicatesMap = new Map>(); + let duplicatesMap: Map> = null; const referenceTypesMap = new Map<{ symbolName: string; file: BscFile; symbolObj: ProvidedSymbol }, Array<{ name: string; namespacedName?: string }>>(); @@ -289,9 +289,12 @@ export class CrossScopeValidator { const symbolIsNamespace = providedTree.getNamespace(symbolName); const isDupe = providedTree.addSymbol(symbolName, { file: file, symbol: symbolObj.symbol }); if (symbolIsNamespace || globalSymbol || isDupe || symbolObj.duplicates.length > 0) { - let dupesSet = duplicatesMap.get(symbolName); + let dupesSet = duplicatesMap?.get(symbolName) ?? null; if (!dupesSet) { dupesSet = new Set<{ file: BrsFile; symbol: BscSymbol }>(); + if (!duplicatesMap) { + duplicatesMap = new Map>(); + } duplicatesMap.set(symbolName, dupesSet); const existing = providedTree.getSymbol(symbolName); if (existing) { @@ -539,7 +542,7 @@ export class CrossScopeValidator { }); const { missingSymbols, duplicatesMap } = this.getIssuesForScope(scope); - if (addDuplicateSymbolDiagnostics) { + if (addDuplicateSymbolDiagnostics && duplicatesMap) { for (const [_flag, dupeSet] of duplicatesMap.entries()) { if (dupeSet.size > 1) { diff --git a/src/bscPlugin/completions/CompletionsProcessor.ts b/src/bscPlugin/completions/CompletionsProcessor.ts index 117ac1578..14581933d 100644 --- a/src/bscPlugin/completions/CompletionsProcessor.ts +++ b/src/bscPlugin/completions/CompletionsProcessor.ts @@ -188,7 +188,7 @@ export class CompletionsProcessor { const tokenKind = currentToken?.kind; //if cursor is after a comment, disable completions - if (this.isPostionInComment(file, position)) { + if (this.isPositionInComment(file, position)) { return emptyResult; } @@ -643,7 +643,7 @@ export class CompletionsProcessor { ]; } - private isPostionInComment(file: BrsFile, position: Position) { + private isPositionInComment(file: BrsFile, position: Position) { const currentToken = file.getCurrentOrNextTokenAt(position); const tokenKind = currentToken?.kind; if (!currentToken) { @@ -654,7 +654,7 @@ export class CompletionsProcessor { } const nextNonComment = file.getNextTokenByPredicate(currentToken, (t: Token) => !AllowedTriviaTokens.includes(t.kind), 1); - const firstComment = nextNonComment?.leadingTrivia.find(t => t.kind === TokenKind.Comment); + const firstComment = nextNonComment?.leadingTrivia?.find(t => t.kind === TokenKind.Comment); if (firstComment && util.comparePosition(position, firstComment?.location?.range.start) >= 0) { return true; } diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 5baee8896..ed0fc3310 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -3467,7 +3467,7 @@ describe('BrsFile', () => { event.file.ast.walk(createVisitor({ AstNode: (node) => { //delete all trivia from the node - for (let i = 0; i < (node.leadingTrivia.length ?? 0); i++) { + for (let i = 0; i < (node.leadingTrivia?.length ?? 0); i++) { delete node.leadingTrivia[i]; } for (let i = 0; i < (node.endTrivia.length ?? 0); i++) { diff --git a/src/lexer/Lexer.spec.ts b/src/lexer/Lexer.spec.ts index f84cb1e7c..b2efeb5fa 100644 --- a/src/lexer/Lexer.spec.ts +++ b/src/lexer/Lexer.spec.ts @@ -1456,7 +1456,7 @@ describe('lexer', () => { return tokens //exclude the explicit triva tokens since they'll be included in the leading/trailing arrays .filter(x => !AllowedTriviaTokens.includes(x.kind)) - .flatMap(x => [...x.leadingTrivia, x]) + .flatMap(x => [...x.leadingTrivia ?? [], x]) .map(x => x.text) .join(''); } @@ -1476,13 +1476,37 @@ describe('lexer', () => { ).to.eql(input); }); + it('tokens only have leadingTrivia array when they have at least one trivia item', () => { + const input = ` + function test( ) + 'comment + print "alpha" + x = 1 + 2 + ' blabla + print x + end function`; + const tokens = Lexer.scan(input).tokens; + let numWithTrivia = 0; + for (const token of tokens) { + if (token.leadingTrivia) { + expect(token.leadingTrivia.length).to.greaterThan(0); + numWithTrivia++; + } else { + expect(token.leadingTrivia).to.be.undefined; + } + + } + expect(numWithTrivia).to.be.lessThan(tokens.length); + }); + + function expectTrivia(text: string, expected: Array<{ text: string; leadingTrivia?: string[]; trailingTrivia?: string[] }>) { const tokens = Lexer.scan(text).tokens.filter(x => !AllowedTriviaTokens.includes(x.kind)); expect( tokens.map(x => { return { text: x.text, - leadingTrivia: x.leadingTrivia.map(x => x.text) + leadingTrivia: x.leadingTrivia?.map(x => x.text) ?? [] }; }) ).to.eql( diff --git a/src/lexer/Lexer.ts b/src/lexer/Lexer.ts index a0ac4b573..63efbd31f 100644 --- a/src/lexer/Lexer.ts +++ b/src/lexer/Lexer.ts @@ -123,7 +123,7 @@ export class Lexer { ? util.createLocation(this.lineBegin, this.columnBegin, this.lineEnd, this.columnEnd + 1, this.uri) : undefined, leadingWhitespace: this.leadingWhitespace, - leadingTrivia: this.leadingTrivia ?? [] + leadingTrivia: this.leadingTrivia.length > 0 ? this.leadingTrivia : undefined }); this.leadingWhitespace = ''; return this; @@ -1079,13 +1079,13 @@ export class Lexer { isReserved: ReservedWords.has(text.toLowerCase()), location: this.locationOf(), leadingWhitespace: this.leadingWhitespace, - leadingTrivia: [] + leadingTrivia: undefined }; if (this.isTrivia(token)) { this.pushTrivia(token); - } else { - token.leadingTrivia.push(...this.leadingTrivia); + } else if (this.leadingTrivia.length > 0) { + token.leadingTrivia = [...this.leadingTrivia]; this.leadingTrivia = []; } this.leadingWhitespace = ''; diff --git a/src/lexer/Token.ts b/src/lexer/Token.ts index e940cc4b2..9e73eed01 100644 --- a/src/lexer/Token.ts +++ b/src/lexer/Token.ts @@ -30,7 +30,7 @@ export interface Token { /** * Any tokens starting on the next line of the previous token, up to the start of this token */ - leadingTrivia: Token[]; + leadingTrivia?: Token[]; } /** diff --git a/src/parser/Statement.ts b/src/parser/Statement.ts index c4004e3ff..b91b6063e 100644 --- a/src/parser/Statement.ts +++ b/src/parser/Statement.ts @@ -928,14 +928,14 @@ export class PrintStatement extends Statement { ] as TranspileResult; //if the first expression has no leading whitespace, add a single space between the `print` and the expression - if (this.expressions.length > 0 && !this.expressions[0].leadingTrivia.find(t => t?.kind === TokenKind.Whitespace)) { + if (this.expressions.length > 0 && !this.expressions[0].leadingTrivia?.find(t => t?.kind === TokenKind.Whitespace)) { result.push(' '); } // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let i = 0; i < this.expressions.length; i++) { const expression = this.expressions[i]; - let leadingWhitespace = expression.leadingTrivia.find(t => t?.kind === TokenKind.Whitespace)?.text; + let leadingWhitespace = expression.leadingTrivia?.find(t => t?.kind === TokenKind.Whitespace)?.text; if (leadingWhitespace) { result.push(leadingWhitespace); //if the previous expression was NOT a separator, and this one is not also, add a space between them @@ -2936,7 +2936,7 @@ export class ClassStatement extends Statement implements TypedefProvider { state.classStatement = this; state.skipLeadingComments = true; //add leading comments - if (statement.leadingTrivia.filter(token => token.kind === TokenKind.Comment).length > 0) { + if ((statement.leadingTrivia?.filter(token => token.kind === TokenKind.Comment) ?? []).length > 0) { result.push( ...state.transpileComments(statement.leadingTrivia), state.indent() diff --git a/src/parser/TranspileState.ts b/src/parser/TranspileState.ts index aead3c2a7..fced4e45d 100644 --- a/src/parser/TranspileState.ts +++ b/src/parser/TranspileState.ts @@ -113,6 +113,9 @@ export class TranspileState { public transpileLeadingCommentsForAstNode(node: { leadingTrivia?: Token[] }) { const leadingTrivia = node?.leadingTrivia ?? []; + if (!leadingTrivia || leadingTrivia.length === 0) { + return []; + } const leadingCommentsSourceNodes = this.transpileComments(leadingTrivia); if (leadingCommentsSourceNodes.length > 0) { // indent in preparation for next text @@ -123,7 +126,10 @@ export class TranspileState { } public transpileLeadingComments(token: TranspileToken) { - const leadingTrivia = (token?.leadingTrivia ?? []); + const leadingTrivia = token?.leadingTrivia ?? []; + if (!leadingTrivia || leadingTrivia.length === 0) { + return []; + } const leadingCommentsSourceNodes = this.transpileComments(leadingTrivia); if (leadingCommentsSourceNodes.length > 0 && token.text) { // indent in preparation for next text diff --git a/src/util.ts b/src/util.ts index 459b3e80c..69f911c49 100644 --- a/src/util.ts +++ b/src/util.ts @@ -792,10 +792,14 @@ export class Util { public pathToUri(filePath: string) { if (!filePath) { return filePath; + } else if (this.pathToURICache.has(filePath)) { + return this.pathToURICache.get(filePath); } else if (this.isUriLike(filePath)) { return filePath; } else { - return URI.file(filePath).toString(); + const uri = URI.file(filePath).toString(); + this.pathToURICache.set(filePath, uri); + return uri; } } @@ -1077,7 +1081,7 @@ export class Util { text: token.text, isReserved: token.isReserved, leadingWhitespace: token.leadingWhitespace, - leadingTrivia: token.leadingTrivia.map(x => this.cloneToken(x)) + leadingTrivia: token.leadingTrivia ? token.leadingTrivia.map(x => this.cloneToken(x)) : undefined } as Token; //handle those tokens that have charCode if ('charCode' in token) { @@ -1839,6 +1843,8 @@ export class Util { // Prevent unbounded growth in long-lived language-server sessions. private standardizePathCacheLimit = 20_000; + private pathToURICache = new Map(); + /** * Converts a path into a standardized format (drive letter to lower, remove extra slashes, use single slash type, resolve relative parts, etc...) */ @@ -1882,7 +1888,8 @@ export class Util { thePath = String.fromCharCode(firstChar + 32) + thePath.slice(1); } } - this.addToStandardizePathCache(originalPath, thePath); + this.standardizePathCache.set(originalPath, thePath); + //this.addToStandardizePathCache(originalPath, thePath); return thePath; } @@ -2536,7 +2543,7 @@ export class Util { public hasLeadingComments(input: Token | AstNode) { const leadingTrivia = isToken(input) ? input?.leadingTrivia : input?.leadingTrivia ?? []; - return !!leadingTrivia.find(t => t?.kind === TokenKind.Comment); + return !!leadingTrivia?.find(t => t?.kind === TokenKind.Comment); } public getLeadingComments(input: Token | AstNode) { From 39c1aae0175d5e5c554e54a9000f66bba0eabb60 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Tue, 12 May 2026 16:24:54 -0300 Subject: [PATCH 3/3] removed cache limit and fixed tests --- src/astUtils/visitors.spec.ts | 2 +- src/util.spec.ts | 13 ------------- src/util.ts | 12 +----------- 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/src/astUtils/visitors.spec.ts b/src/astUtils/visitors.spec.ts index 316313b9b..a8d6e4382 100644 --- a/src/astUtils/visitors.spec.ts +++ b/src/astUtils/visitors.spec.ts @@ -1332,7 +1332,7 @@ describe('astUtils visitors', () => { file.ast.walk(createVisitor({ AstNode: (node) => { - const endNodeComments = node.endTrivia.filter(t => t.kind === TokenKind.Comment); + const endNodeComments = node.endTrivia?.filter(t => t.kind === TokenKind.Comment) ?? []; comments.push(...endNodeComments); } }), { diff --git a/src/util.spec.ts b/src/util.spec.ts index 40a535f43..5c8e728b7 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -1706,19 +1706,6 @@ describe('util', () => { }); }); - it('bounds cache growth by clearing when limit is reached', () => { - util['standardizePathCacheLimit'] = 2; - - util.standardizePath('source/a.bs'); - util.standardizePath('source/b.bs'); - expect(util['standardizePathCache'].size).to.equal(2); - - util.standardizePath('source/c.bs'); - - expect(util['standardizePathCache'].size).to.equal(1); - expect(util['standardizePathCache'].has('source/c.bs')).to.be.true; - }); - describe('windows paths on unix', () => { beforeEach(() => { isWindows = false; diff --git a/src/util.ts b/src/util.ts index 69f911c49..e325a361a 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1840,8 +1840,6 @@ export class Util { private isWindows = process.platform === 'win32'; private standardizePathCache = new Map(); - // Prevent unbounded growth in long-lived language-server sessions. - private standardizePathCacheLimit = 20_000; private pathToURICache = new Map(); @@ -1863,7 +1861,7 @@ export class Util { if (/^virtual:[\/\\]/i.test(thePath)) { // Strip the `virtual:` prefix, normalize slashes, then re-add the prefix thePath = 'virtual:/' + thePath.slice(8).replace(/^[\/\\]+/, '').replace(/[\/\\]+/g, '/').toLowerCase(); - this.addToStandardizePathCache(originalPath, thePath); + this.standardizePathCache.set(originalPath, thePath); return thePath; } @@ -1889,17 +1887,9 @@ export class Util { } } this.standardizePathCache.set(originalPath, thePath); - //this.addToStandardizePathCache(originalPath, thePath); return thePath; } - private addToStandardizePathCache(key: string, value: string) { - if (this.standardizePathCache.size >= this.standardizePathCacheLimit) { - this.standardizePathCache.clear(); - } - this.standardizePathCache.set(key, value); - } - /** * Given a Diagnostic or BsDiagnostic, return a deep clone of the diagnostic. * @param diagnostic the diagnostic to clone