fix: improve event-loop promise state tracking #52108

Uzlopak · 2024-03-15T22:31:58Z

Fixes #43655
Fixes #34851
Superseeds #34862

@nodejs/performance

Can you help me with the changes in my last commit? I adapted changes from #34862. This should improve the performance of rejected promises. Maybe you have some concerns regarding the the changes. I am not that convinced from the last commit.

Ty

lib/internal/process/promises.js

mcollina

lgtm

mcollina · 2024-03-18T09:40:10Z

I think we should remove multipleResolves event in a follow-up PR.

nodejs-github-bot · 2024-03-18T09:47:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57801/

nodejs-github-bot · 2024-03-19T15:01:52Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57840/

Uzlopak · 2024-03-20T21:56:52Z

@mcollina
@anonrig

Is there something i have to do?

aduh95 · 2024-03-20T22:02:56Z

Yes, the added test is flaky:

not ok 2290 parallel/test-promise-unhandled-issue-43655
  ---
  duration_ms: 1031.49800
  severity: fail
  exitcode: 1
  stack: |-
    node:internal/process/promises:417
        triggerUncaughtException(err, true /* fromPromise */);
        ^
    
    AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
    
      assert.ok(Date.now() - time0 < 10)
    
        at test (/Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/test/parallel/test-promise-unhandled-issue-43655.js:23:10) {
      generatedMessage: true,
      code: 'ERR_ASSERTION',
      actual: false,
      expected: true,
      operator: '=='
    }
    
    Node.js v22.0.0-pre
  ...

nodejs-github-bot · 2024-03-20T22:02:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57865/

Uzlopak · 2024-03-21T02:16:35Z

It should be now less flaky.

mcollina

lgtm

nodejs-github-bot · 2024-04-07T15:51:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58178/

mertcanaltin

LGTM

lib/internal/process/promises.js

test/fixtures/errors/unhandled_promise_trace_warnings.snapshot

nodejs-github-bot · 2024-04-23T16:37:27Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58645/

Uzlopak · 2024-04-24T12:01:51Z

Are the test failures related to my patch? I have no osx-arm machine to check it.

BridgeAR · 2024-04-24T12:28:44Z

@Uzlopak they do not seem to be related

lib/internal/process/promises.js

Uzlopak · 2024-05-06T03:24:08Z

@BridgeAR
PTAL

benjamingr

Still lgtm

nodejs-github-bot · 2024-05-06T11:05:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58983/

nodejs-github-bot · 2024-05-06T17:41:58Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58989/

Uzlopak · 2024-05-06T19:35:38Z

Whoa!!!! the CI is green. the CI is green!

nodejs-github-bot · 2024-05-06T20:16:23Z

Landed in be8d64e

PR-URL: nodejs#52108 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

PR-URL: #52108 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

PR-URL: nodejs#52108 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

nodejs-github-bot added needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Mar 15, 2024

Uzlopak changed the title ~~chore: improve event-loop~~ chore: improve event-loop promise state tracking Mar 15, 2024

Uzlopak force-pushed the improve-event-loop branch 3 times, most recently from b4b0a4c to d221af4 Compare March 16, 2024 03:02

aduh95 reviewed Mar 16, 2024

View reviewed changes

lib/internal/process/promises.js Outdated Show resolved Hide resolved

Uzlopak mentioned this pull request Mar 17, 2024

tracking of promise rejections nodejs/performance#153

Closed

Uzlopak changed the title ~~chore: improve event-loop promise state tracking~~ fix: improve event-loop promise state tracking Mar 17, 2024

Uzlopak requested a review from aduh95 March 17, 2024 17:07

mcollina approved these changes Mar 18, 2024

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 18, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 18, 2024

anonrig approved these changes Mar 19, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

Uzlopak force-pushed the improve-event-loop branch from 13e0e13 to 014d1b5 Compare March 21, 2024 02:13

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 7, 2024

mcollina approved these changes Apr 7, 2024

View reviewed changes

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 7, 2024

mertcanaltin approved these changes Apr 12, 2024

View reviewed changes

benjamingr reviewed Apr 12, 2024

View reviewed changes

lib/internal/process/promises.js Show resolved Hide resolved

benjamingr reviewed Apr 12, 2024

View reviewed changes

test/fixtures/errors/unhandled_promise_trace_warnings.snapshot Show resolved Hide resolved

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 23, 2024

Uzlopak force-pushed the improve-event-loop branch 2 times, most recently from 71b3502 to 691b724 Compare April 24, 2024 12:27

BridgeAR reviewed Apr 24, 2024

View reviewed changes

lib/internal/process/promises.js Outdated Show resolved Hide resolved

Uzlopak force-pushed the improve-event-loop branch from 691b724 to a5dc9a3 Compare May 6, 2024 03:22

process: improve event-loop

6fd930b

Uzlopak force-pushed the improve-event-loop branch from a5dc9a3 to 6fd930b Compare May 6, 2024 03:52

benjamingr approved these changes May 6, 2024

View reviewed changes

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2024

mertcanaltin approved these changes May 6, 2024

View reviewed changes

H4ad added the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2024

nodejs-github-bot merged commit be8d64e into nodejs:main May 6, 2024

Uzlopak deleted the improve-event-loop branch May 6, 2024 20:18

Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this pull request May 8, 2024

process: improve event-loop

900fb89

PR-URL: nodejs#52108 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

targos pushed a commit that referenced this pull request May 8, 2024

process: improve event-loop

0bbc62c

PR-URL: #52108 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

targos mentioned this pull request May 13, 2024

v22.2.0 release proposal #52971

Merged

marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024

process: improve event-loop

121ea13

PR-URL: #52108 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

marco-ippolito mentioned this pull request Jun 17, 2024

v20.15.0 proposal #53486

Merged

bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024

process: improve event-loop

de90235

PR-URL: nodejs#52108 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

Uh oh!

fix: improve event-loop promise state tracking #52108

fix: improve event-loop promise state tracking #52108

Uh oh!

Conversation

Uzlopak commented Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina commented Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 18, 2024

Uh oh!

nodejs-github-bot commented Mar 19, 2024

Uh oh!

Uzlopak commented Mar 20, 2024

Uh oh!

aduh95 commented Mar 20, 2024

Uh oh!

nodejs-github-bot commented Mar 20, 2024

Uh oh!

This comment was marked as outdated.

Uzlopak commented Mar 21, 2024

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 7, 2024

Uh oh!

mertcanaltin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 23, 2024

Uh oh!

Uzlopak commented Apr 24, 2024

Uh oh!

BridgeAR commented Apr 24, 2024

Uh oh!

Uh oh!

Uzlopak commented May 6, 2024

Uh oh!

benjamingr left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 6, 2024

Uh oh!

nodejs-github-bot commented May 6, 2024

Uh oh!

Uzlopak commented May 6, 2024

Uh oh!

nodejs-github-bot commented May 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Uzlopak commented Mar 15, 2024 •

edited

Loading

mcollina commented Mar 18, 2024 •

edited

Loading