From 61b0246674863a4e0b1a957fffe58061f0e6360f Mon Sep 17 00:00:00 2001 From: "Sakamoto, Kazunori" Date: Thu, 26 Dec 2024 09:55:17 +0900 Subject: [PATCH] fix: make persistence cache work on different methods with same arguments --- src/memoize.ts | 15 ++++++++++----- tests/unit/memoize.test.ts | 35 +++++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/memoize.ts b/src/memoize.ts index fd2a8d7..e24d8e5 100644 --- a/src/memoize.ts +++ b/src/memoize.ts @@ -1,5 +1,7 @@ import { sha3_512 } from './hash.js'; +let globalCounter = 0; + /** * A memoization decorator/function that caches the results of method/getter/function calls to improve performance. * This decorator/function can be applied to methods and getters in a class as a decorator, and functions without context as a function. @@ -36,7 +38,7 @@ export const memoize = memoizeFactory(); export function memoizeFactory({ cacheDuration = Number.POSITIVE_INFINITY, caches, - calcHash = (thisArg: unknown, args: unknown) => sha3_512(JSON.stringify([thisArg, args])), + calcHash = (thisArg: unknown, counter: number, args: unknown) => sha3_512(JSON.stringify([thisArg, counter, args])), maxCachedArgsSize = 100, persistCache, removeCache, @@ -44,7 +46,7 @@ export function memoizeFactory({ }: { maxCachedArgsSize?: number; cacheDuration?: number; - calcHash?: (thisArg: unknown, args: unknown) => string; + calcHash?: (thisArg: unknown, counter: number, args: unknown) => string; caches?: Map[]; persistCache?: (hash: string, currentTime: number, value: unknown) => void; tryReadingCache?: (hash: string) => [number, unknown] | undefined; @@ -56,13 +58,14 @@ export function memoizeFactory({ | ClassMethodDecoratorContext Return> | ClassGetterDecoratorContext ): (this: This, ...args: Args) => Return { + const counter = globalCounter++; if (context?.kind === 'getter') { const cache = new Map(); caches?.push(cache); return function (this: This): Return { console.log(`Entering getter ${String(context.name)}.`); - const key = calcHash(this, []); + const key = calcHash(this, counter, []); const now = Date.now(); // Check in-memory cache first @@ -138,9 +141,11 @@ export function memoizeFactory({ caches?.push(cache); return function (this: This, ...args: Args): Return { - console.log(`Entering ${context ? `method ${String(context.name)}` : 'function'}(${calcHash(this, args)}).`); + console.log( + `Entering ${context ? `method ${String(context.name)}` : 'function'}(${calcHash(this, counter, args)}).` + ); - const key = calcHash(this, args); + const key = calcHash(this, counter, args); const now = Date.now(); // Check in-memory cache first diff --git a/tests/unit/memoize.test.ts b/tests/unit/memoize.test.ts index 0946f09..2adff48 100644 --- a/tests/unit/memoize.test.ts +++ b/tests/unit/memoize.test.ts @@ -18,6 +18,11 @@ describe('memory cache', () => { return base + getNextInteger(); } + @memoize + nextString(base = 0): string { + return String(base + getNextInteger()); + } + abstract get count(): number; } @@ -66,6 +71,15 @@ describe('memory cache', () => { await expect(asyncErrorFunction()).rejects.toThrow('Test error'); }); + test('memoize method per method', () => { + expect(random1.nextInteger()).toBe(random1.nextInteger()); + expect(random1.nextInteger(100)).toBe(random1.nextInteger(100)); + expect(random1.nextString()).toBe(random1.nextString()); + expect(random1.nextString(100)).toBe(random1.nextString(100)); + expect(random1.nextInteger()).not.toBe(random1.nextString()); + expect(random1.nextInteger(100)).not.toBe(random1.nextString(100)); + }); + test('memoize method per instance', () => { expect(random1.nextInteger()).not.toBe(random2.nextInteger()); expect(random1.nextInteger(100)).not.toBe(random2.nextInteger(100)); @@ -126,13 +140,15 @@ describe('persistent cache', () => { } const caches: Map[] = []; - const nextIntegerWithPersistence = memoizeFactory({ + const memoize = memoizeFactory({ caches, persistCache, tryReadingCache, removeCache, cacheDuration: 200, - })((base: number = 0): number => base + getNextInteger()); + }); + const nextIntegerWithPersistence = memoize((base: number = 0): number => base + getNextInteger()); + const nextIntegerWithPersistence2 = memoize((base: number = 0): number => base + getNextInteger()); function clearCache(): void { for (const cache of caches) { @@ -145,15 +161,18 @@ describe('persistent cache', () => { clearCache(); }); - test('should use persistent cache', () => { + test('persist cache per method', () => { const initial = nextIntegerWithPersistence(100); clearCache(); expect(nextIntegerWithPersistence(100)).toBe(initial); expect(persistentStore.size).toBe(1); + + expect(nextIntegerWithPersistence2(100)).not.toBe(initial); + expect(persistentStore.size).toBe(2); }); - test('should remove expired cache', async () => { + test('remove expired cache', async () => { const initial = nextIntegerWithPersistence(100); clearCache(); @@ -164,7 +183,7 @@ describe('persistent cache', () => { expect(persistentStore.size).toBe(1); }); - test('should handle multiple cache entries', () => { + test('handle multiple cache entries', () => { const value1 = nextIntegerWithPersistence(100); const value2 = nextIntegerWithPersistence(200); clearCache(); @@ -174,7 +193,7 @@ describe('persistent cache', () => { expect(nextIntegerWithPersistence(200)).toBe(value2); }); - test('should remove oldest cache entry when maxCachedArgsSize is reached', () => { + test('remove oldest cache entry when maxCachedArgsSize is reached', () => { const withSizeLimit = memoizeFactory({ persistCache, tryReadingCache, @@ -214,7 +233,7 @@ describe('error handling in cache operations', () => { cacheDuration: 200, })((base: number = 0): number => base + getNextInteger()); - test('should ignore errors in persistCache, tryReadingCache and removeCache', () => { + test('ignore errors in persistCache, tryReadingCache and removeCache', () => { expect(() => nextIntegerWithErrorHandling(100)).not.toThrow(); }); }); @@ -235,7 +254,7 @@ describe('async error handling in cache operations', () => { cacheDuration: 200, })((base: number = 0): number => base + getNextInteger()); - test('should ignore errors in async persistCache, non-async tryReadingCache and async removeCache', async () => { + test('ignore errors in async persistCache, non-async tryReadingCache and async removeCache', async () => { expect(() => nextIntegerWithAsyncErrorHandling(100)).not.toThrow(); }); });