Skip to content

Commit

Permalink
[4/n] Add TTL option to WeakMapMemoize (#27228)
Browse files Browse the repository at this point in the history
## Summary & Motivation

Add option to allow cache entries to be automatically evicted after a
set time.

## How I Tested These Changes

jest + used the functionality for elastic search filter support in the
asset selection input in cloud
  • Loading branch information
salazarm authored Jan 21, 2025
1 parent 21395c6 commit 6c7c7b9
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,90 @@ describe('weakMapMemoize', () => {
expect(spy).toHaveBeenCalledTimes(6);
});
});

describe('weakMapMemoize with TTL', () => {
beforeEach(() => {
jest.useFakeTimers();
});

afterEach(() => {
jest.useRealTimers();
});

// Test 16: Function with TTL option
it('should evict cache entries after TTL expires', () => {
const spy = jest.fn((a: number, b: number) => a + b);
const memoizedAdd = weakMapMemoize(spy, {maxEntries: 5, ttl: 2}); // TTL of 2 seconds

// Initial calls - should cache results
expect(memoizedAdd(1, 2)).toBe(3);
expect(memoizedAdd(1, 2)).toBe(3);
expect(memoizedAdd(2, 3)).toBe(5);
expect(spy).toHaveBeenCalledTimes(2); // Two unique calls

// Advance time by 1 second - cache should still be valid
jest.advanceTimersByTime(1000);
expect(memoizedAdd(1, 2)).toBe(3);
expect(memoizedAdd(2, 3)).toBe(5);
expect(spy).toHaveBeenCalledTimes(2); // No new calls

// Advance time by another 2 seconds (total 3 seconds) - cache should expire
jest.advanceTimersByTime(2000);
expect(memoizedAdd(1, 2)).toBe(3);
expect(memoizedAdd(2, 3)).toBe(5);
expect(spy).toHaveBeenCalledTimes(4); // Cache evicted, two new calls
});

// Test 17: Function with TTL and LRU eviction
it('should handle TTL expiration alongside LRU eviction', () => {
const spy = jest.fn((a: number) => a * 2);
const memoizedFn = weakMapMemoize(spy, {maxEntries: 2, ttl: 2}); // TTL of 2 seconds

expect(memoizedFn(1)).toBe(2);
expect(memoizedFn(2)).toBe(4);
expect(memoizedFn(3)).toBe(6); // Evicts 1

expect(spy).toHaveBeenCalledTimes(3);

jest.advanceTimersByTime(1000);

// Still in cache
expect(memoizedFn(2)).toBe(4);
expect(memoizedFn(3)).toBe(6);
expect(spy).toHaveBeenCalledTimes(3);

expect(memoizedFn(1)).toBe(2);
expect(spy).toHaveBeenCalledTimes(4);

// Advance time by 2 seonds, TTL expires
jest.advanceTimersByTime(2000);

// All cache entries should have expired
expect(memoizedFn(1)).toBe(2);
expect(memoizedFn(2)).toBe(4);
expect(memoizedFn(3)).toBe(6);
expect(spy).toHaveBeenCalledTimes(7);
});

// Test 18: Function with TTL option and no maxEntries
it('should evict cache entries after TTL when maxEntries is not set', () => {
const spy = jest.fn((a: number) => a * a);
const memoizedFn = weakMapMemoize(spy, {ttl: 1}); // TTL of 1 second

expect(memoizedFn(2)).toBe(4);
expect(memoizedFn(3)).toBe(9);
expect(spy).toHaveBeenCalledTimes(2);

// Advance time by 500ms - cache should still be valid
jest.advanceTimersByTime(500);
expect(memoizedFn(2)).toBe(4);
expect(memoizedFn(3)).toBe(9);
expect(spy).toHaveBeenCalledTimes(2);

// Advance time by another 600ms (total 1.1 seconds) - cache should expire
jest.advanceTimersByTime(600);
expect(memoizedFn(2)).toBe(4);
expect(memoizedFn(3)).toBe(9);
expect(spy).toHaveBeenCalledTimes(4); // New calls after eviction
});
});
38 changes: 36 additions & 2 deletions js_modules/dagster-ui/packages/ui-core/src/util/weakMapMemoize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ type AnyFunction = (...args: any[]) => any;

interface WeakMapMemoizeOptions {
maxEntries?: number; // Optional limit for cached entries
ttl?: number; // Time-To-Live in seconds
}

interface CacheNode {
Expand All @@ -14,6 +15,7 @@ interface CacheNode {
parentKey?: any;
lruKey?: any; // Reference to the key in the LRU cache
childCount: number; // Number of child nodes
timeoutId?: ReturnType<typeof setTimeout>; // Timer for TTL eviction
}

/**
Expand Down Expand Up @@ -41,6 +43,11 @@ function recursivelyDeleteParent(cacheNode: CacheNode) {
parent.map.delete(parentKey);
}

// Clear the TTL timer if set
if (cacheNode.timeoutId) {
clearTimeout(cacheNode.timeoutId);
}

// Decrement the parent's child count
parent.childCount--;

Expand All @@ -53,14 +60,14 @@ function recursivelyDeleteParent(cacheNode: CacheNode) {

/**
* Memoizes a function using nested Maps and WeakMaps based on the arguments.
* Optionally limits the number of cached entries using an LRU cache.
* Optionally limits the number of cached entries using an LRU cache and sets TTL for cache entries.
* Handles both primitive and object arguments efficiently.
* @param fn - The function to memoize.
* @param options - Optional settings for memoization.
* @returns A memoized version of the function.
*/
export function weakMapMemoize<T extends AnyFunction>(fn: T, options?: WeakMapMemoizeOptions): T {
const {maxEntries} = options || {};
const {maxEntries, ttl} = options || {};

// Initialize the root cache node
const cacheRoot: CacheNode = {
Expand All @@ -79,6 +86,11 @@ export function weakMapMemoize<T extends AnyFunction>(fn: T, options?: WeakMapMe
// Remove the cached result
delete cacheNode.result;

// Clear the TTL timer if set
if (cacheNode.timeoutId) {
clearTimeout(cacheNode.timeoutId);
}

// If there are no child nodes, proceed to remove this node from its parent
if (cacheNode.childCount === 0 && cacheNode.parent && cacheNode.parentKey !== undefined) {
recursivelyDeleteParent(cacheNode);
Expand Down Expand Up @@ -145,6 +157,28 @@ export function weakMapMemoize<T extends AnyFunction>(fn: T, options?: WeakMapMe
// Cache the result
currentCache.result = result;

// If TTL is specified, set a timeout to evict the cache entry
if (ttl) {
currentCache.timeoutId = setTimeout(() => {
// Remove the result
delete currentCache.result;

// Remove from LRU if applicable
if (lruCache && currentCache.lruKey) {
lruCache.del(currentCache.lruKey);
}

// Recursively delete parent nodes if necessary
if (
currentCache.childCount === 0 &&
currentCache.parent &&
currentCache.parentKey !== undefined
) {
recursivelyDeleteParent(currentCache);
}
}, ttl * 1000); // Convert seconds to milliseconds
}

// If LRU cache is enabled, manage the cache entries
if (lruCache && !currentCache.lruKey) {
const cacheEntryKey = Symbol();
Expand Down

1 comment on commit 6c7c7b9

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-ogvjrhb1v-elementl.vercel.app

Built with commit 6c7c7b9.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.