-
Notifications
You must be signed in to change notification settings - Fork 2
Add plimit and increase cache size #23
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
base: main
Are you sure you want to change the base?
Conversation
ivankalinovski
commented
Feb 2, 2025
- Add plimit to protect from spikes
- Incease cache size to better performance in spikes.
- Add plimit to protect from spikes - Incease cache size to better performance in spikes.
Add to node-options experimental-vm-modules to support plimit on jest
|
|
||
| const initPLimit = async () => { | ||
| if (!pLimit) { | ||
| const module = await import('p-limit') |
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.
this assumes ESM, is it working? don't we use CJS?
| const execAsync = async (object, filter, {enableEnv = false, throwOnError = false} = {}) => { | ||
| try { | ||
| const data = await nativeJq.execAsync(JSON.stringify(object), formatFilter(filter, {enableEnv})) | ||
| const limit = await initPLimit() |
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.
- what do you think about passing the plimit object from outside and then the service can create the p-limit optimized to its needs.
For example:
import pLimit from 'p-limi'
const limit = pLimit()
execAsync(object, filter, {pLimit})- Don't we need to initialize the pLimit earlier?
| const module = await import('p-limit') | ||
| pLimit = module.default(currentLimit) | ||
| } | ||
| return pLimit |
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.
Should it be like this so we won't initialize the pLimit object on every request?
| return pLimit | |
| return pLimit(currentLimit) |
| "install": "node-gyp rebuild", | ||
| "prepack": "npm run test", | ||
| "test": "jest" | ||
| "test": "NODE_OPTIONS='--experimental-vm-modules' jest" |
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.
We are mixing cjs and esm, better to stick with one until we do the change to esm