FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

vm: refactor to avoid unsafe array iteration by aduh95 · Pull Request #36752 · nodejs/node · GitHub

/ node Public

vm: refactor to avoid unsafe array iteration - #36752

Merged
aduh95 merged 1 commit into
nodejs:masterfrom
aduh95:vm-array-iteration
Jan 13, 2021
Merged

vm: refactor to avoid unsafe array iteration#36752
aduh95 merged 1 commit into
nodejs:masterfrom
aduh95:vm-array-iteration

Conversation

aduh95 commented Jan 3, 2021

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

aduh95 added vm Issues and PRs related to the vm subsystem. needs-benchmark-ci PR that need a benchmark CI run. labels Jan 3, 2021

aduh95 commented Jan 3, 2021

Copy link
Copy Markdown
Contributor Author

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/869/ (queued, will 404 until it starts)

aduh95 commented Jan 3, 2021

Copy link
Copy Markdown
Contributor Author

No perf regression or improvement

Details
                                                                    confidence improvement accuracy (*)    (**)   (***)
 vm/create-context.js n=100                                                        -1.06 %       ±8.00% ±10.65% ±13.88%
 vm/run-in-context.js withSigintListener=0 breakOnSigint=0 n=1                     -1.67 %       ±7.09%  ±9.43% ±12.27%
 vm/run-in-context.js withSigintListener=0 breakOnSigint=1 n=1                      2.73 %       ±7.58% ±10.11% ±13.21%
 vm/run-in-context.js withSigintListener=1 breakOnSigint=0 n=1                      1.23 %       ±6.48%  ±8.65% ±11.31%
 vm/run-in-context.js withSigintListener=1 breakOnSigint=1 n=1                     -2.97 %       ±4.42%  ±5.89%  ±7.66%
 vm/run-in-this-context.js withSigintListener=0 breakOnSigint=0 n=1                -2.35 %       ±5.64%  ±7.50%  ±9.77%
 vm/run-in-this-context.js withSigintListener=0 breakOnSigint=1 n=1                 5.30 %       ±6.35%  ±8.48% ±11.10%
 vm/run-in-this-context.js withSigintListener=1 breakOnSigint=0 n=1                 1.00 %       ±5.77%  ±7.69% ±10.03%
 vm/run-in-this-context.js withSigintListener=1 breakOnSigint=1 n=1                -3.33 %       ±4.76%  ±6.33%  ±8.24%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 9 comparisons, you can thus
expect the following amount of false-positive results:
  0.45 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.09 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)
++ mv output030121-121422.csv /w/bnch-comp

aduh95 added review wanted PRs that need reviews. and removed needs-benchmark-ci PR that need a benchmark CI run. labels Jan 3, 2021
aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 12, 2021
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 13, 2021

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

PR-URL: nodejs#36752
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 force-pushed the vm-array-iteration branch from 9b615d8 to e70e793 Compare January 13, 2021 14:49

aduh95 commented Jan 13, 2021

Copy link
Copy Markdown
Contributor Author

Landed in e70e793

aduh95 merged commit e70e793 into nodejs:master Jan 13, 2021
aduh95 deleted the vm-array-iteration branch January 13, 2021 14:50
ruyadorno pushed a commit that referenced this pull request Jan 22, 2021
PR-URL: #36752
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno mentioned this pull request Jan 22, 2021
targos pushed a commit that referenced this pull request May 25, 2021
PR-URL: #36752
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request May 30, 2021
PR-URL: #36752
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
PR-URL: #36752
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
PR-URL: #36752
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, and a CI started. review wanted PRs that need reviews. vm Issues and PRs related to the vm subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL