Skip to content

Conversation

@aduh95
Copy link
Contributor

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

@nodejs-github-botnodejs-github-bot added the process Issues and PRs related to the process subsystem. label Nov 21, 2020
@aduh95aduh95 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-actionsgithub-actionsbot 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 23, 2020

@Trott
Copy link
Member

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Pranshu Srivastava <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@TrottTrottforce-pushed the process-primordials branch from 642f480 to d4501b1CompareNovember 24, 2020 13:28
@TrottTrott merged commit d4501b1 into nodejs:masterNov 24, 2020
@Trott
Copy link
Member

Landed in d4501b1

@aduh95aduh95 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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Pranshu Srivastava <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@danielleadamsdanielleadams mentioned this pull request Dec 7, 2020
@aduh95
Copy link
ContributorAuthor

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

@aduh95
Copy link
ContributorAuthor

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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.processIssues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@aduh95@nodejs-github-bot@Trott@jasnell@targos@rexagod