Skip to content
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

lib: speed up MessageEvent creation internally #52951

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

KhafraDev
Copy link
Member

createFastMessageEvent can be used when the arguments passed to the MessageEvent constructor do not need to be validated.

Refs: nodejs/undici#3170
Refs: #52370 (review)

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. worker Issues and PRs related to Worker support. labels May 12, 2024
@KhafraDev KhafraDev force-pushed the use-fast-create-messageevent branch from 98377de to 43aa76e Compare May 12, 2024 02:54
@KhafraDev KhafraDev changed the title perf: speed up MessageEvent creation internally lib: speed up MessageEvent creation internally May 12, 2024
@benjamingr
Copy link
Member

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

LGTM with benchmark ci

@benjamingr
Copy link
Member

Benchmarks look great:

10:03:02                                                                         confidence improvement accuracy (*)   (**)  (***)
10:03:02 worker/atomics-wait.js n=10000000                                                       0.05 %       ±0.59% ±0.79% ±1.03%
10:03:02 worker/bench-eventlooputil.js method='ELU_passed' n=1000000                             0.64 %       ±0.67% ±0.90% ±1.18%
10:03:02 worker/bench-eventlooputil.js method='ELU_simple' n=1000000                             0.07 %       ±0.52% ±0.69% ±0.90%
10:03:02 worker/echo.js n=100000 sendsPerBroadcast=1 payload='object' workers=1                  0.12 %       ±0.69% ±0.92% ±1.19%
10:03:02 worker/echo.js n=100000 sendsPerBroadcast=1 payload='string' workers=1                  0.18 %       ±0.71% ±0.95% ±1.24%
10:03:02 worker/echo.js n=100000 sendsPerBroadcast=10 payload='object' workers=1        ***      2.73 %       ±0.71% ±0.95% ±1.24%
10:03:02 worker/echo.js n=100000 sendsPerBroadcast=10 payload='string' workers=1        ***      3.17 %       ±1.13% ±1.51% ±1.96%
10:03:02 worker/messageport.js n=1000000 style='eventemitter' payload='object'                  -0.06 %       ±0.36% ±0.48% ±0.63%
10:03:02 worker/messageport.js n=1000000 style='eventemitter' payload='string'                  -0.54 %       ±0.55% ±0.74% ±0.97%
10:03:02 worker/messageport.js n=1000000 style='eventtarget' payload='object'           ***     46.14 %       ±0.81% ±1.09% ±1.43%
10:03:02 worker/messageport.js n=1000000 style='eventtarget' payload='string'           ***     90.95 %       ±0.54% ±0.72% ±0.93%
10:03:02   0.55 false positives, when considering a   5% risk acceptance (*, **, ***),
10:03:02   0.11 false positives, when considering a   1% risk acceptance (**, ***),
10:03:02   0.01 false positives, when considering a 0.1% risk acceptance (***)


@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels May 12, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 12, 2024
@nodejs-github-bot
Copy link
Collaborator

@KhafraDev KhafraDev added the request-ci Add this label to start a Jenkins CI on a PR. label May 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 15, 2024
@KhafraDev KhafraDev added the request-ci Add this label to start a Jenkins CI on a PR. label May 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 15, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants