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

worker: refactor to avoid unsafe array iteration #37346

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Feb 13, 2021

No description provided.

@aduh95 aduh95 added the worker Issues and PRs related to Worker support. label Feb 13, 2021
@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 13, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 13, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/36100/
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/946/

No perf regressions found.
                                                                       confidence improvement accuracy (*)   (**)  (***)
worker/atomics-wait.jsn=10000000                                                      -1.57 %       ±2.85% ±3.80% ±4.97%
worker/bench-eventlooputil.jsmethod='ELU_passed' n=1000000                            -0.53 %       ±1.82% ±2.42% ±3.15%
worker/bench-eventlooputil.jsmethod='ELU_simple' n=1000000                             0.05 %       ±2.41% ±3.21% ±4.20%
worker/echo.jsn=100000 sendsPerBroadcast=10 payload='object' workers=1                 2.83 %       ±4.91% ±6.54% ±8.51%
worker/echo.jsn=100000 sendsPerBroadcast=10 payload='string' workers=1                 0.41 %       ±4.96% ±6.60% ±8.60%
worker/echo.jsn=100000 sendsPerBroadcast=1 payload='object' workers=1                  2.81 %       ±3.48% ±4.63% ±6.03%
worker/echo.jsn=100000 sendsPerBroadcast=1 payload='string' workers=1                 -1.52 %       ±4.02% ±5.35% ±6.97%
worker/messageport.jsn=1000000 style='eventemitter' payload='object'                  -1.22 %       ±2.64% ±3.53% ±4.61%
worker/messageport.jsn=1000000 style='eventemitter' payload='string'                  -2.82 %       ±4.11% ±5.47% ±7.14%
worker/messageport.jsn=1000000 style='eventtarget' payload='object'                    1.90 %       ±4.03% ±5.38% ±7.04%
worker/messageport.jsn=1000000 style='eventtarget' payload='string'            **      5.21 %       ±3.24% ±4.32% ±5.64%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 11 comparisons, you can thus
expect the following amount of false-positive results:
  0.55 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.11 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

PR-URL: nodejs#37346
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aduh95 aduh95 merged commit 0f29587 into nodejs:master Feb 15, 2021
@aduh95
Copy link
Contributor Author

aduh95 commented Feb 15, 2021

Landed in 0f29587

@aduh95 aduh95 deleted the worker-array-iteration branch February 15, 2021 22:09
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
PR-URL: #37346
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Feb 16, 2021
targos pushed a commit that referenced this pull request May 27, 2021
PR-URL: #37346
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request May 30, 2021
PR-URL: #37346
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
PR-URL: #37346
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
PR-URL: #37346
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants