Implement BufferCollection for Document caching#64
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a BufferCollection singleton for caching document buffers to improve performance when reading document contents multiple times. The caching system includes size limits and automatic eviction to prevent memory issues.
- Adds BufferCollection class with size-aware caching and eviction logic
- Integrates buffer caching into document reading functions (readDocx, readPdf, readTxt)
- Clears cache on file save in VS Code server to ensure fresh content
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/poml/base.tsx | Implements BufferCollection singleton with size calculation and cache management |
| packages/poml/components/document.tsx | Adds readBufferCached function and integrates caching into document readers |
| packages/poml-vscode/lsp/server.ts | Clears buffer cache on document save to ensure fresh content |
| packages/poml/tests/components.test.tsx | Tests buffer caching behavior and size limit enforcement |
| packages/poml/tests/base.test.tsx | Tests recursive size calculation functionality |
| if (t === 'number' || t === 'boolean' || t === 'bigint') { | ||
| return 8; | ||
| } | ||
| if (t === 'object') { |
There was a problem hiding this comment.
Adding objects to the visited set before checking if they're arrays could cause issues. Arrays should be handled separately from generic objects to avoid incorrect size calculations.
| if (t === 'object') { | |
| if (t === 'object') { | |
| if (Array.isArray(value)) { | |
| visited.add(value); | |
| let size = 0; | |
| for (const element of value) { | |
| try { | |
| size += calculateSize(element, visited); | |
| } catch { | |
| // ignore | |
| } | |
| } | |
| visited.delete(value); | |
| return size; | |
| } |
| } catch { | ||
| // ignore |
There was a problem hiding this comment.
The try-catch block silently ignores all errors when calculating property sizes. This could mask important issues. Consider logging errors or being more specific about which exceptions to catch.
| } catch { | |
| // ignore | |
| } catch (error) { | |
| console.error(`Error calculating size for key "${key}":`, error); | |
| // Continue processing other keys despite the error |
| return cached.value; | ||
| } | ||
| const buf = fs.readFileSync(abs); | ||
| BufferCollection.set(key, buf, buf.length, stat.mtimeMs); |
There was a problem hiding this comment.
Passing buf.length as the size parameter is redundant since calculateSize() will be called anyway when size is provided. Either remove the size parameter or optimize to avoid double calculation.
| BufferCollection.set(key, buf, buf.length, stat.mtimeMs); | |
| BufferCollection.set(key, buf, stat.mtimeMs); |
|
|
||
| test('skip cache when over limit', () => { | ||
| BufferCollection.clear(); | ||
| const inst = (BufferCollection as any).instance as any; |
There was a problem hiding this comment.
Using 'as any' to access private members breaks encapsulation. Consider adding a public method for testing or using a test-specific subclass to avoid casting to any.
| BufferCollection.clear(); | ||
| const obj = { data: { arr: [1, 2, 'abc'] }, extra: Buffer.alloc(4) }; | ||
| BufferCollection.set('obj', obj); | ||
| const inst = (BufferCollection as any).instance as any; |
There was a problem hiding this comment.
Using 'as any' to access private members breaks encapsulation. Consider adding a public method for testing or using a test-specific subclass to avoid casting to any.
Summary
BufferCollectionsingleton for caching buffers<Document>contentsBufferCollectionand skip oversize valuescalculateSizecalculationTesting
npm run build-webviewnpm run build-cli(with warnings)npm run lint(with warnings)npm testpython -m pytest python/testsxvfb-run -a npm run compilenpm run build-extensionxvfb-run -a npm run test-vscodehttps://chatgpt.com/codex/tasks/task_e_68887d1df1e0832eaacfa92e007b2bce