-
Notifications
You must be signed in to change notification settings - Fork 459
Add additional memoization to different UI levels #8184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lib/shared/src/utils.ts
Outdated
export function memoize<T extends (...args: any[]) => any>( | ||
func: T | ||
): (...args: Parameters<T>) => ReturnType<T> { | ||
let lastArguments: any[] | null = null | ||
let lastCalculatedValue: ReturnType<T> | null = null | ||
|
||
return (...args: Parameters<T>): ReturnType<T> => { | ||
if (isEqual(lastArguments, args)) { | ||
return lastCalculatedValue as ReturnType<T> | ||
} | ||
|
||
lastArguments = args | ||
lastCalculatedValue = func(args) | ||
|
||
return lastCalculatedValue as ReturnType<T> | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think lodash has a memoize function as well... does that not work for our purposes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it does have memoize
function, but this function gathers all arguments in key-value cache store, which will produce another place which GC can free up, so I remember only the last result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I tested this the same way as I tested my branch: #8183
and I get exactly the same (improved vs main
) results
For me this does not fix the memory leak but makes an app much quicker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds memoisation to different UI components to avoid unnecessary react renders (potentially it should improve memory allocation during LLM responses on the chat client)
Test plan