Skip to content

Conversation

@ivankalinovski
Copy link
Contributor

  • Add plimit to protect from spikes
  • Incease cache size to better performance in spikes.

Ivan Kalinovski added 2 commits February 2, 2025 11:11
- 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')
Copy link
Collaborator

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()
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 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})
  1. Don't we need to initialize the pLimit earlier?

const module = await import('p-limit')
pLimit = module.default(currentLimit)
}
return pLimit
Copy link
Collaborator

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?

Suggested change
return pLimit
return pLimit(currentLimit)

"install": "node-gyp rebuild",
"prepack": "npm run test",
"test": "jest"
"test": "NODE_OPTIONS='--experimental-vm-modules' jest"
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants