Repository navigation
Problem with GC (million promise rejections) #43655
Description
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.
Activity
bnoordhuis commentedon Jul 3, 2022
I can confirm the issue and I speculate it's caused by how V8 internally tracks rejected promises.
Here is a reduced test case:
What's remarkable is that performance is not linear. A lower value like 30_000 is 2x slower than 36_923.
Slayer95 commentedon Jul 3, 2022
I see a few similarities with #29385. Maybe they are related?
benjamingr commentedon Jul 28, 2022
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 commentedon Jul 28, 2022
@bnoordhuis
I suspect that since we push to a WeakMap it does something differently when it reaches a threshold.
Comment out
setPromiseRejectCallbackand see if that "resolves" the performance difference.Oh and @NikolayMakhonin :
The promise impl was entirely swapped and replaced/improved in Chrome since Chrome 49 :)
benjamingr commentedon Jul 28, 2022
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 commentedon Nov 8, 2022
@benjamingr Do you still want to open that PR?
benjamingr commentedon Nov 11, 2022
@NikolayMakhonin can you confirm that the fix (working with --unhandled-rejections=none) would address the issue from your point of view?
itzmanish commentedon Mar 19, 2023
I think this may be related to issue #47158 which I recently created.
NikolayMakhonin commentedon Apr 29, 2023
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 commentedon Apr 29, 2023
I don't think that change was ever merged (or PR'd.)