Problem with GC (million promise rejections)

[NikolayMakhonin]

Version

16.15.1

Platform

Microsoft Windows NT 10.0.19042.0 x64

Subsystem

No response

What steps will reproduce the bug?

Just run it in the Node.JS <= 18:

(I have tested it on node v14.19.3, v16.15.1, v18.2.0)

test.js

function delay(time) {
  return new Promise((resolve) => {
    setTimeout(resolve, time)
  })
}

async function test() {
  for (let i = 0; i < 1000000; i++) {
    await new Promise((resolve, reject) => {
      reject('value')
    })
      .then(() => {}, () => {})

    // if (i % 10000 === 0) {
    //  await delay(100)
    // }
  }

  console.log('OK')

  const time0 = Date.now()
  await delay(0)
  console.log('Real delay time: ' + (Date.now() - time0))
}

test()

The test function will generate 1 million of Promise rejections. And after finish it will hang. It seems like the garbage collection called on idle, and it takes very long time.
If you replace the reject with the resolve, then everything will work quickly.
You can also uncomment the periodically delay, and it will work quickly.

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

No response

What do you see instead?

Work without hungs

Additional information

I can reproduce it in the Chromium 49 but it work correctly in the Chrome latest. It looks like this bug has been fixed in the Chrome.

This bug is easy to work around, but now I have the same bug with 30000 call of worker functions. I can't describe it yet, because I need to research it before. I think maybe the promise rejection bug will expose a wider problem and I won't have to create a new issue.

[bnoordhuis]

I can confirm the issue and I speculate it's caused by how V8 internally tracks rejected promises.

Here is a reduced test case:

// Flags: --predictable
(async () => {
  for (let i = 0; i < 36_293; i++) // fast - 36_294 is 3x slower
    await new Promise((_, g) => g()).then(() => {}, () => {}) // not .catch
})()

What's remarkable is that performance is not linear. A lower value like 30_000 is 2x slower than 36_923.

[Slayer95]

I see a few similarities with #29385. Maybe they are related?

[benjamingr]

Rejections are significantly more costly than fulfillment (e.g. because of rejection tracking) which is "by design".

Exceptional control flow prioritizes reliability and debuggability over performance and whenever we discussed what to make faster in promises the consensus was always that error flow performance isn't important since errors are rare.

[benjamingr]

@bnoordhuis

A lower value like 30_000 is 2x slower than 36_923.

I suspect that since we push to a WeakMap it does something differently when it reaches a threshold.

Comment out setPromiseRejectCallback and see if that "resolves" the performance difference.

Oh and @NikolayMakhonin :

I can reproduce it in the Chromium 49 but it work correctly in the Chrome latest.

The promise impl was entirely swapped and replaced/improved in Chrome since Chrome 49 :)

[benjamingr]

I'll open a PR to not save promises in the map if the unhandled rejection mode is "none". Note the cost of not saving rejections is that you will not get notified of unhandled rejections if that helps

[targos]

@benjamingr Do you still want to open that PR?

[benjamingr]

@NikolayMakhonin can you confirm that the fix (working with --unhandled-rejections=none) would address the issue from your point of view?

[itzmanish]

I think this may be related to issue #47158 which I recently created.

[NikolayMakhonin]

I tested this again on Node 16.20.0, the issue persists.

I also tried --unhandled-rejections=none, as @benjamingr suggested above, but it didn't help.

Now I'm getting this error not in the tests, but in a real application that synchronize millions of files between hard drives. There is the same thing: Node hangs, CPU is fully loaded, but JavaScript is doing nothing.

[bnoordhuis]

I also tried --unhandled-rejections=none, as @benjamingr suggested above, but it didn't help.

I don't think that change was ever merged (or PR'd.)