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

process: refactor to use more primordials #36212

Merged
merged 1 commit into from
Nov 24, 2020

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Nov 21, 2020

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

@nodejs-github-bot nodejs-github-bot added the process Issues and PRs related to the process subsystem. label Nov 21, 2020
lib/internal/process/per_thread.js Outdated Show resolved Hide resolved
lib/internal/process/per_thread.js Outdated Show resolved Hide resolved
lib/internal/process/per_thread.js Outdated Show resolved Hide resolved
@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 Nov 21, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2020
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Nov 24, 2020

Benchmark results seem fine:

12:03:01                                                       confidence improvement accuracy (*)    (**)   (***)
12:03:01  process/bench-env.js operation='delete' n=1000000                   -0.35 %       ±3.26%  ±4.34%  ±5.68%
12:03:01  process/bench-env.js operation='enumerate' n=1000000                -2.38 %       ±3.19%  ±4.24%  ±5.53%
12:03:01  process/bench-env.js operation='get' n=1000000                       2.11 %       ±4.48%  ±5.96%  ±7.77%
12:03:01  process/bench-env.js operation='query' n=1000000                     0.93 %       ±7.18%  ±9.56% ±12.44%
12:03:01  process/bench-env.js operation='set' n=1000000                *     -2.33 %       ±2.26%  ±3.01%  ±3.94%
12:03:01  process/bench-hrtime.js type='bigint' n=1000000                     -3.00 %       ±4.19%  ±5.60%  ±7.34%
12:03:01  process/bench-hrtime.js type='diff' n=1000000                       -3.87 %       ±5.66%  ±7.54%  ±9.84%
12:03:01  process/bench-hrtime.js type='raw' n=1000000                  *      5.24 %       ±4.72%  ±6.30%  ±8.23%
12:03:01  process/memoryUsage.js n=100000                                      1.99 %       ±2.32%  ±3.09%  ±4.03%
12:03:01  process/next-tick-breadth-args.js n=10000000                  *      4.68 %       ±4.50%  ±5.99%  ±7.79%
12:03:01  process/next-tick-breadth.js n=10000000                             -0.42 %       ±3.72%  ±4.95%  ±6.44%
12:03:01  process/next-tick-depth-args.js n=7000000                            1.22 %       ±2.74%  ±3.66%  ±4.79%
12:03:01  process/next-tick-depth.js n=7000000                                -1.78 %       ±4.47%  ±5.98%  ±7.86%
12:03:01  process/next-tick-exec-args.js n=4000000                             3.66 %      ±12.52% ±16.67% ±21.72%
12:03:01  process/next-tick-exec.js n=4000000                                 -6.66 %       ±9.38% ±12.49% ±16.27%
12:03:01  process/queue-microtask-breadth.js n=400000                          1.04 %       ±2.14%  ±2.85%  ±3.72%
12:03:01  process/queue-microtask-depth.js n=1200000                          -3.65 %       ±6.20%  ±8.25% ±10.74%
12:03:01  process/resourceUsage.js n=100000                                   -1.88 %       ±2.71%  ±3.60%  ±4.69%
12:03:01 
12:03:02 Be aware that when doing many comparisons the risk of a false-positive
12:03:02 result increases. In this case there are 18 comparisons, you can thus
12:03:02 expect the following amount of false-positive results:
12:03:02   0.90 false positives, when considering a   5% risk acceptance (*, **, ***),
12:03:02   0.18 false positives, when considering a   1% risk acceptance (**, ***),
12:03:02   0.02 false positives, when considering a 0.1% risk acceptance (***)

PR-URL: nodejs#36212
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott Trott merged commit d4501b1 into nodejs:master Nov 24, 2020
@Trott
Copy link
Member

Trott commented Nov 24, 2020

Landed in d4501b1

@aduh95 aduh95 deleted the process-primordials branch November 24, 2020 13:46
danielleadams pushed a commit that referenced this pull request Dec 7, 2020
PR-URL: #36212
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request Dec 7, 2020
@aduh95
Copy link
Contributor Author

aduh95 commented Dec 28, 2020

This should not land to LTS lines unless #36660 is included.

@aduh95
Copy link
Contributor Author

aduh95 commented Jan 7, 2021

Does anyone on this thread want to review #36660 so it can be included in the next v15 release? This PR has leaked SafeSet to user-land, we probably want to fix that…

$ node -p 'Object.getPrototypeOf(process.allowedNodeEnvironmentFlags)'
SafeSet [Set] {}

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. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants