diff --git a/.jules/bolt.md b/.jules/bolt.md new file mode 100644 index 0000000..ebd4d54 --- /dev/null +++ b/.jules/bolt.md @@ -0,0 +1,3 @@ +## 2024-05-24 - [Avoid dynamic array allocations on hot cache paths] +**Learning:** Using `[a, b, c].filter(Boolean).join(':')` to dynamically generate cache keys incurs unnecessary array allocation and loop processing overhead, which degrades performance when executed frequently in a hot path like `fetch`. Precomputing the static portion of the key (prefix + version) limits the overhead significantly in Node.js/V8. +**Action:** Always prefer precomputing static values and simple string concatenation (or template literals) for hot paths that run on every cache operation. diff --git a/src/backends/index.ts b/src/backends/index.ts index 20ba43b..3f96b8e 100644 --- a/src/backends/index.ts +++ b/src/backends/index.ts @@ -42,22 +42,26 @@ function getGlobalRedisClient(): Redis { lazyConnect: true, // Don't connect immediately }); + const client = globalRedisClient; + // Set up proper error handling - globalRedisClient.on('error', (error) => { + client.on('error', (error) => { // Only log connection errors, don't throw unhandled rejections if (process.env.NODE_ENV !== 'test') { + // eslint-disable-next-line no-console console.warn('Redis connection error:', error.message); } }); - globalRedisClient.on('connect', () => { + client.on('connect', () => { if (process.env.NODE_ENV !== 'test') { + // eslint-disable-next-line no-console console.log('Redis connected successfully'); } }); // Handle connection promise - connectionPromise = globalRedisClient.connect().then(() => globalRedisClient!).catch((error) => { + connectionPromise = client.connect().then(() => client).catch((error) => { // Reset the promise on error so it can be retried connectionPromise = null; const connectionError = new CacheConnectionError( @@ -66,8 +70,9 @@ function getGlobalRedisClient(): Redis { // In test environment, don't throw unhandled rejections if (process.env.NODE_ENV === 'test') { + // eslint-disable-next-line no-console console.warn('Redis connection failed in test environment:', connectionError.message); - return globalRedisClient!; + return client; } throw connectionError; diff --git a/src/cache/createCacheHandler.ts b/src/cache/createCacheHandler.ts index 70f42de..5c9bceb 100644 --- a/src/cache/createCacheHandler.ts +++ b/src/cache/createCacheHandler.ts @@ -63,12 +63,14 @@ export function createCacheHandler( const l1Cache = new Map(); const L1_CACHE_TTL = 1000; // 1 second TTL for L1 cache + // Precompute the static part of the key to avoid array allocation, filter, and join on every fetch + const basePrefix = prefix && version ? `${prefix}:${version}:` : prefix ? `${prefix}:` : version ? `${version}:` : ''; + /** * Get the fully qualified key with prefix and version */ const getFullKey = (key: string): string => { - const parts = [prefix, version, key].filter(Boolean); - return parts.join(':'); + return `${basePrefix}${key}`; }; /** @@ -253,11 +255,20 @@ export function createCacheHandler( }; // Clean up L1 cache periodically - setInterval(cleanupL1Cache, 5000); // Every 5 seconds + const cleanupInterval = setInterval(cleanupL1Cache, 5000); // Every 5 seconds + + /** + * Clean up resources when the handler is destroyed + */ + const destroy = () => { + clearInterval(cleanupInterval); + l1Cache.clear(); + }; return { fetch, backend, getFullKey, + destroy, }; } \ No newline at end of file diff --git a/src/types/index.ts b/src/types/index.ts index 154bc7e..de9c7a0 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -121,4 +121,9 @@ export interface CacheHandler { * @returns The prefixed key */ getFullKey(key: string): string; + + /** + * Destroy the cache handler, cleaning up any resources such as intervals and L1 cache + */ + destroy?: () => void; } diff --git a/test/cache/createCacheHandler.test.ts b/test/cache/createCacheHandler.test.ts index 7098c97..07f9b73 100644 --- a/test/cache/createCacheHandler.test.ts +++ b/test/cache/createCacheHandler.test.ts @@ -424,5 +424,52 @@ describe('createCacheHandler', () => { }); expect(result).toBe('value'); }); + + it('should clean up expired L1 cache entries automatically', async () => { + vi.useFakeTimers(); + + const timerHandler = createCacheHandler({ + backend, + prefix: 'test', + }); + + // Add to L1 cache + await timerHandler.fetch('test-key', async () => 'value'); + + // Wait for cleanup interval to run (5s) + vi.advanceTimersByTime(5100); + + // Cleanup should have run, clearing the expired item + + // Destroy handler + if (timerHandler.destroy) { + timerHandler.destroy(); + } + + vi.useRealTimers(); + }); + + it('should clean up resources when destroy is called', async () => { + const destroyHandler = createCacheHandler({ + backend, + prefix: 'test', + }); + + // Populate L1 cache by fetching + await destroyHandler.fetch('test-key', async () => 'value'); + + // Destroy should not throw and should clean up intervals and L1 cache + expect(() => { + if (destroyHandler.destroy) { + destroyHandler.destroy(); + } + }).not.toThrow(); + + // We can't directly check the interval is cleared without mocking timers, + // but we can verify it doesn't error when we try to fetch again + // after the L1 cache was cleared. + const result = await destroyHandler.fetch('test-key-2', async () => 'value2'); + expect(result).toBe('value2'); + }); }); }); \ No newline at end of file