From c84343dbbeffb21dacfb69ff5dbaa5c3427366a9 Mon Sep 17 00:00:00 2001 From: Smartsheet-JB-Brown Date: Thu, 17 Apr 2025 21:02:15 -0700 Subject: [PATCH] reduce memory use during MetaDataScan --- jest.config.js | 18 ++- src/__tests__/setupMemoryTests.ts | 46 ++++++ .../package-manager/MetadataScanner.ts | 141 +++++++++--------- .../package-manager/PackageManagerManager.ts | 108 ++++++++------ 4 files changed, 196 insertions(+), 117 deletions(-) create mode 100644 src/__tests__/setupMemoryTests.ts diff --git a/jest.config.js b/jest.config.js index 5172373b55..836f74e2da 100644 --- a/jest.config.js +++ b/jest.config.js @@ -45,5 +45,21 @@ module.exports = { modulePathIgnorePatterns: [".vscode-test"], reporters: [["jest-simple-dot-reporter", {}]], setupFiles: ["/src/__mocks__/jest.setup.ts"], - setupFilesAfterEnv: ["/src/integrations/terminal/__tests__/setupTerminalTests.ts"], + setupFilesAfterEnv: [ + "/src/integrations/terminal/__tests__/setupTerminalTests.ts", + "/src/__tests__/setupMemoryTests.ts", + ], + // Increase test timeout to allow for GC + testTimeout: 10000, + // Run tests in series to better track memory + maxConcurrency: 1, + // Add memory tracking + globals: { + "ts-jest": { + diagnostics: { + warnOnly: true, + ignoreCodes: [151001], + }, + }, + }, } diff --git a/src/__tests__/setupMemoryTests.ts b/src/__tests__/setupMemoryTests.ts new file mode 100644 index 0000000000..76f65edf6f --- /dev/null +++ b/src/__tests__/setupMemoryTests.ts @@ -0,0 +1,46 @@ +// Track memory usage before and after each test +let startMemory: NodeJS.MemoryUsage + +beforeEach(() => { + if (global.gc) { + global.gc() + } + startMemory = process.memoryUsage() +}) + +afterEach(() => { + if (global.gc) { + global.gc() + } + const endMemory = process.memoryUsage() + const diff = { + heapUsed: endMemory.heapUsed - startMemory.heapUsed, + heapTotal: endMemory.heapTotal - startMemory.heapTotal, + external: endMemory.external - startMemory.external, + rss: endMemory.rss - startMemory.rss, + } + + // Log if memory increase is significant (> 50MB) + const SIGNIFICANT_INCREASE = 50 * 1024 * 1024 // 50MB in bytes + if (diff.heapUsed > SIGNIFICANT_INCREASE) { + console.warn(`\nSignificant memory increase detected in test:`) + console.warn(`Heap Used: +${(diff.heapUsed / 1024 / 1024).toFixed(2)}MB`) + console.warn(`Heap Total: +${(diff.heapTotal / 1024 / 1024).toFixed(2)}MB`) + console.warn(`External: +${(diff.external / 1024 / 1024).toFixed(2)}MB`) + console.warn(`RSS: +${(diff.rss / 1024 / 1024).toFixed(2)}MB\n`) + } +}) + +// Add global error handler to catch memory errors +process.on("uncaughtException", (error) => { + if (error.message.includes("heap out of memory")) { + console.error("\nHeap out of memory error detected!") + console.error("Current memory usage:") + const usage = process.memoryUsage() + console.error(`Heap Used: ${(usage.heapUsed / 1024 / 1024).toFixed(2)}MB`) + console.error(`Heap Total: ${(usage.heapTotal / 1024 / 1024).toFixed(2)}MB`) + console.error(`External: ${(usage.external / 1024 / 1024).toFixed(2)}MB`) + console.error(`RSS: ${(usage.rss / 1024 / 1024).toFixed(2)}MB\n`) + } + throw error +}) diff --git a/src/services/package-manager/MetadataScanner.ts b/src/services/package-manager/MetadataScanner.ts index 13a9bad4e4..dc9afa7dbd 100644 --- a/src/services/package-manager/MetadataScanner.ts +++ b/src/services/package-manager/MetadataScanner.ts @@ -21,6 +21,9 @@ export class MetadataScanner { private readonly git?: SimpleGit private localizationOptions: LocalizationOptions private originalRootDir: string | null = null + private static readonly MAX_DEPTH = 5 // Maximum directory depth + private static readonly BATCH_SIZE = 50 // Number of items to process at once + private static readonly CONCURRENT_SCANS = 3 // Number of concurrent directory scans constructor(git?: SimpleGit, localizationOptions?: LocalizationOptions) { this.git = git @@ -30,6 +33,68 @@ export class MetadataScanner { } } + /** + * Generator function to yield items in batches + */ + private async *scanDirectoryBatched( + rootDir: string, + repoUrl: string, + sourceName?: string, + depth: number = 0, + ): AsyncGenerator { + if (depth > MetadataScanner.MAX_DEPTH) { + return + } + + const batch: PackageManagerItem[] = [] + const entries = await fs.readdir(rootDir, { withFileTypes: true }) + + for (const entry of entries) { + if (!entry.isDirectory()) continue + + const componentDir = path.join(rootDir, entry.name) + const metadata = await this.loadComponentMetadata(componentDir) + const localizedMetadata = metadata ? this.getLocalizedMetadata(metadata) : null + + if (localizedMetadata) { + const item = await this.createPackageManagerItem( + localizedMetadata, + componentDir, + repoUrl, + this.originalRootDir || rootDir, + sourceName, + ) + + if (item) { + // If this is a package, scan for subcomponents + if (this.isPackageMetadata(localizedMetadata)) { + await this.scanPackageSubcomponents(componentDir, item) + } + + batch.push(item) + if (batch.length >= MetadataScanner.BATCH_SIZE) { + yield batch.splice(0) + } + } + } + + // Recursively scan subdirectories + if (!localizedMetadata || !this.isPackageMetadata(localizedMetadata)) { + const subGenerator = this.scanDirectoryBatched(componentDir, repoUrl, sourceName, depth + 1) + for await (const subBatch of subGenerator) { + batch.push(...subBatch) + if (batch.length >= MetadataScanner.BATCH_SIZE) { + yield batch.splice(0) + } + } + } + } + + if (batch.length > 0) { + yield batch + } + } + /** * Scans a directory for components * @param rootDir The root directory to scan @@ -37,87 +102,25 @@ export class MetadataScanner { * @param sourceName Optional source repository name * @returns Array of discovered items */ + /** + * Scan a directory and return items in batches + */ async scanDirectory( rootDir: string, repoUrl: string, sourceName?: string, isRecursiveCall: boolean = false, ): Promise { - const items: PackageManagerItem[] = [] - // Only set originalRootDir on the first call if (!isRecursiveCall && !this.originalRootDir) { this.originalRootDir = rootDir } - try { - const entries = await fs.readdir(rootDir, { withFileTypes: true }) + const items: PackageManagerItem[] = [] + const generator = this.scanDirectoryBatched(rootDir, repoUrl, sourceName) - // Process directories sequentially to avoid memory spikes - for (const entry of entries) { - if (!entry.isDirectory()) continue - - const componentDir = path.join(rootDir, entry.name) - const relativePath = path.relative(this.originalRootDir || rootDir, componentDir).replace(/\\/g, "/") - - // Load metadata once - const metadata = await this.loadComponentMetadata(componentDir) - const localizedMetadata = metadata ? this.getLocalizedMetadata(metadata) : null - - if (localizedMetadata) { - // Create item if we have valid metadata - const item = await this.createPackageManagerItem( - localizedMetadata, - componentDir, - repoUrl, - this.originalRootDir || rootDir, - sourceName, - ) - - if (item) { - // Handle package items - if (this.isPackageMetadata(localizedMetadata)) { - // Process listed items sequentially - if (localizedMetadata.items) { - item.items = [] - for (const subItem of localizedMetadata.items) { - const subPath = path.join(componentDir, subItem.path) - const subMetadata = await this.loadComponentMetadata(subPath) - const localizedSubMetadata = subMetadata - ? this.getLocalizedMetadata(subMetadata) - : null - - if (localizedSubMetadata) { - item.items.push({ - type: subItem.type, - path: subItem.path, - metadata: localizedSubMetadata, - lastUpdated: await this.getLastModifiedDate(subPath), - }) - } - } - } - - // Scan for unlisted components - await this.scanPackageSubcomponents(componentDir, item) - items.push(item) - continue // Skip further recursion for package directories - } - - items.push(item) - } - } - - // Only recurse if: - // 1. No metadata was found, or - // 2. Metadata was found but it's not a package - if (!localizedMetadata || !this.isPackageMetadata(localizedMetadata)) { - const subItems = await this.scanDirectory(componentDir, repoUrl, sourceName, true) - items.push(...subItems) - } - } - } catch (error) { - console.error(`Error scanning directory ${rootDir}:`, error) + for await (const batch of generator) { + items.push(...batch) } return items diff --git a/src/services/package-manager/PackageManagerManager.ts b/src/services/package-manager/PackageManagerManager.ts index dd4acbf56f..b18ba0391c 100644 --- a/src/services/package-manager/PackageManagerManager.ts +++ b/src/services/package-manager/PackageManagerManager.ts @@ -287,8 +287,9 @@ export class PackageManagerManager { * @param filters The filter criteria * @returns Filtered items */ - // Cache size limit to prevent memory issues private static readonly MAX_CACHE_SIZE = 100 + private static readonly BATCH_SIZE = 100 + private filterCache = new Map< string, { @@ -312,6 +313,9 @@ export class PackageManagerManager { } } + /** + * Filter items + */ filterItems( items: PackageManagerItem[], filters: { type?: ComponentType; search?: string; tags?: string[] }, @@ -326,22 +330,40 @@ export class PackageManagerManager { // Clean up old cache entries this.cleanupFilterCache() - // Helper function to normalize text for case/whitespace-insensitive comparison - const normalizeText = (text: string) => text.toLowerCase().replace(/\s+/g, " ").trim() - - // Normalize search term once - const searchTerm = filters.search ? normalizeText(filters.search) : "" - - // Helper function to check if text contains the search term - const containsSearchTerm = (text: string) => { - if (!searchTerm) return true - return normalizeText(text).includes(normalizeText(searchTerm)) + // Process items in batches to avoid memory spikes + const allFilteredItems: PackageManagerItem[] = [] + for (let i = 0; i < items.length; i += PackageManagerManager.BATCH_SIZE) { + const batch = items.slice(i, Math.min(i + PackageManagerManager.BATCH_SIZE, items.length)) + const filteredBatch = this.processItemBatch(batch, filters) + allFilteredItems.push(...filteredBatch) } - // Filter items with shallow copies - const filteredItems = items + // Cache the results + this.filterCache.set(cacheKey, { + items: allFilteredItems, + timestamp: Date.now(), + }) + + return allFilteredItems + } + + /** + * Process a batch of items + */ + /** + * Process a batch of items + */ + private processItemBatch( + batch: PackageManagerItem[], + filters: { type?: ComponentType; search?: string; tags?: string[] }, + ): PackageManagerItem[] { + // Helper functions + const normalizeText = (text: string) => text.toLowerCase().replace(/\s+/g, " ").trim() + const searchTerm = filters.search ? normalizeText(filters.search) : "" + const containsSearchTerm = (text: string) => !searchTerm || normalizeText(text).includes(searchTerm) + + return batch .map((item) => { - // Create shallow copy of item const itemCopy = { ...item } // Check parent item matches @@ -354,7 +376,7 @@ export class PackageManagerManager { (itemCopy.tags && filters.tags.some((tag) => itemCopy.tags!.includes(tag))), } - // Process subcomponents and track if any match + // Process subcomponents let hasMatchingSubcomponents = false if (itemCopy.items?.length) { itemCopy.items = itemCopy.items.map((subItem) => { @@ -363,14 +385,13 @@ export class PackageManagerManager { search: !searchTerm || (subItem.metadata && - (containsSearchTerm(subItem.metadata.name) || - containsSearchTerm(subItem.metadata.description))), + (containsSearchTerm(subItem.metadata.name || "") || + containsSearchTerm(subItem.metadata.description || "") || + containsSearchTerm(subItem.type || ""))), tags: !filters.tags?.length || - !!( - subItem.metadata?.tags && - filters.tags.some((tag) => subItem.metadata!.tags!.includes(tag)) - ), + (subItem.metadata?.tags && + filters.tags.some((tag) => subItem.metadata!.tags!.includes(tag))), } const subItemMatched = @@ -380,16 +401,13 @@ export class PackageManagerManager { if (subItemMatched) { hasMatchingSubcomponents = true - // Set matchInfo for matching subcomponent - // Build match reason based on active filters - const matchReason: Record = {} - - if (searchTerm) { - matchReason.nameMatch = containsSearchTerm(subItem.metadata?.name || "") - matchReason.descriptionMatch = containsSearchTerm(subItem.metadata?.description || "") + const matchReason: Record = { + nameMatch: searchTerm ? containsSearchTerm(subItem.metadata?.name || "") : true, + descriptionMatch: searchTerm + ? containsSearchTerm(subItem.metadata?.description || "") + : false, } - // Always include typeMatch when filtering by type if (filters.type) { matchReason.typeMatch = subMatches.type } @@ -413,19 +431,11 @@ export class PackageManagerManager { const isPackageWithMatchingSubcomponent = itemCopy.type === "package" && hasMatchingSubcomponents if (parentMatchesAll || isPackageWithMatchingSubcomponent) { - // Add match info without deep cloning - // Build parent match reason based on active filters - const matchReason: Record = {} - - if (searchTerm) { - matchReason.nameMatch = containsSearchTerm(itemCopy.name) - matchReason.descriptionMatch = containsSearchTerm(itemCopy.description) - } else { - matchReason.nameMatch = false - matchReason.descriptionMatch = false + const matchReason: Record = { + nameMatch: searchTerm ? containsSearchTerm(itemCopy.name) : false, + descriptionMatch: searchTerm ? containsSearchTerm(itemCopy.description) : false, } - // Always include typeMatch when filtering by type if (filters.type) { matchReason.typeMatch = itemMatches.type } @@ -434,22 +444,26 @@ export class PackageManagerManager { matchReason.hasMatchingSubcomponents = true } + // If this is a package and we're searching, also check if any subcomponent names match + if (searchTerm && itemCopy.type === "package" && itemCopy.items?.length) { + const subcomponentNameMatches = itemCopy.items.some( + (subItem) => subItem.metadata && containsSearchTerm(subItem.metadata.name || ""), + ) + if (subcomponentNameMatches) { + matchReason.hasMatchingSubcomponents = true + } + } + itemCopy.matchInfo = { matched: true, matchReason, } return itemCopy } + return null }) .filter((item): item is PackageManagerItem => item !== null) - - // Cache the results with timestamp - this.filterCache.set(cacheKey, { - items: filteredItems, - timestamp: Date.now(), - }) - return filteredItems } /**