-
Notifications
You must be signed in to change notification settings - Fork 1
feat(): lru memoization for js worker #466
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
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.
Pull Request Overview
This PR introduces LRU-based memoization for the JavaScript worker to reduce database calls and improve performance when processing source maps and function context extraction. The implementation uses a custom decorator with configurable cache size and TTL.
- Adds a new
@memoizedecorator with LRU cache support and hashing strategies - Updates crypto utility to support Blake2b hashing for large JSON objects
- Applies memoization to
loadSourceMapFileandgetFunctionContextmethods
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/memoize/index.ts | New LRU-based memoization decorator with hash and concat key strategies |
| lib/utils/crypto.ts | Enhanced hash function with Blake2b support and configurable digest formats |
| workers/javascript/src/index.ts | Applied memoization to source map loading and function context methods |
| workers/javascript/tests/index.test.ts | Added comprehensive tests for memoization behavior and GridFS mocking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Peter <[email protected]>
Co-authored-by: Peter <[email protected]>
Problem
We need to cache large json objects to decrease number db calls and to reduce cpu and memory usage
Solution
Use lru cache in band with hashed objects as a keys
Why lru?
Why crypto module is changed?
blake2b512cause it is prefered for large json objects hashing and it is presented in default js crypto lib seeTodo
lodash.memoizelib could be replaced with this implementationRisks
all of the risks could be solved by the configuration of the cache max elements (i've set to 50 just for example, should be descussed), maybe we can use
maxSizeof thelru-cacheand allow dynamically decide whenever to add a new object to the cache or notalso ttl value should be discussed, but it is placed in env