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

lib: improve async_context_frame structure by Qard · Pull Request #54239 · nodejs/node · GitHub

/ node Public

lib: improve async_context_frame structure - #54239

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Qard:als-rewrite-followup
Aug 9, 2024
ConversationCommits (1)ChecksFiles changed
Merged

lib: improve async_context_frame structure#54239
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Qard:als-rewrite-followup

Conversation

Qard commented Aug 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

This is a followup to #48528 to deal with some unresolved feedback and to try a prototype-swap improvement to speed up AsyncResource performance a bit. Here's the benchmark numbers:

                                                                                          confidence improvement accuracy (*)   (**)  (***)
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=10          ***      0.65 %       ±0.20% ±0.27% ±0.34%
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=100                  0.05 %       ±0.24% ±0.32% ±0.40%
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=1000        ***      1.07 %       ±0.27% ±0.35% ±0.45%
async_hooks/async-local-storage-getstore-nested-run.js n=10000 storageCount=1                     *     -0.81 %       ±0.66% ±0.87% ±1.11%
async_hooks/async-local-storage-getstore-nested-run.js n=10000 storageCount=10                          -0.45 %       ±0.65% ±0.85% ±1.09%
async_hooks/async-local-storage-getstore-nested-run.js n=10000 storageCount=100                          0.46 %       ±0.67% ±0.88% ±1.13%
async_hooks/async-local-storage-propagate-asyncresource.js n=1000 storageCount=0                ***      4.35 %       ±0.68% ±0.90% ±1.14%
async_hooks/async-local-storage-propagate-asyncresource.js n=1000 storageCount=1                ***      5.27 %       ±0.58% ±0.77% ±0.98%
async_hooks/async-local-storage-propagate-asyncresource.js n=1000 storageCount=10               ***      1.28 %       ±0.63% ±0.83% ±1.06%
async_hooks/async-local-storage-propagate-asyncresource.js n=1000 storageCount=100               **      0.37 %       ±0.23% ±0.30% ±0.38%
async_hooks/async-local-storage-propagate-promise.js n=100000 storageCount=0                    ***      0.59 %       ±0.34% ±0.45% ±0.57%
async_hooks/async-local-storage-propagate-promise.js n=100000 storageCount=1                    ***      0.27 %       ±0.15% ±0.20% ±0.25%
async_hooks/async-local-storage-propagate-promise.js n=100000 storageCount=10                            0.11 %       ±0.12% ±0.16% ±0.20%
async_hooks/async-local-storage-propagate-promise.js n=100000 storageCount=100                          -0.17 %       ±0.19% ±0.26% ±0.33%
async_hooks/async-local-storage-run.js n=10000000                                                        0.29 %       ±0.39% ±0.51% ±0.66%

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

Qard added doc Issues and PRs related to the documentations. diag-agenda Issues and PRs to discuss during the meetings of the diagnostics working group. async_local_storage AsyncLocalStorage request-ci Add this label to start a Jenkins CI on a PR. labels Aug 7, 2024
nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 7, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2024

nodejs-github-bot commented Aug 7, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60919/

Qard force-pushed the als-rewrite-followup branch from 57238f0 to 185ec61 Compare August 7, 2024 02:40

codecov Bot commented Aug 7, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.09%. Comparing base (d1229ee) to head (604d429).
Report is 514 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/async_context_frame.js 92.30% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54239      +/-   ##
==========================================
- Coverage   87.09%   87.09%   -0.01%     
==========================================
  Files         647      647              
  Lines      181836   181778      -58     
  Branches    34917    34882      -35     
==========================================
- Hits       158376   158320      -56     
- Misses      16743    16766      +23     
+ Partials     6717     6692      -25
Files with missing lines Coverage Δ
lib/async_hooks.js 99.65% <100.00%> (ø)
...nternal/async_local_storage/async_context_frame.js 78.72% <ø> (ø)
lib/internal/process/task_queues.js 100.00% <100.00%> (ø)
lib/internal/timers.js 99.45% <100.00%> (ø)
src/api/async_resource.cc 95.83% <100.00%> (ø)
lib/internal/async_context_frame.js 79.72% <92.30%> (-20.28%) ⬇️

... and 50 files with indirect coverage changes

Qard force-pushed the als-rewrite-followup branch from 185ec61 to cb6d5e6 Compare August 7, 2024 05:16
Qard added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2024

nodejs-github-bot commented Aug 7, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60922/

nodejs-github-bot commented Aug 7, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60948/

Flarna reviewed Aug 8, 2024
View reviewed changes
Comment thread lib/internal/process/task_queues.js Outdated
Flarna reviewed Aug 8, 2024
View reviewed changes
Comment thread lib/internal/async_context_frame.js Outdated
Qard force-pushed the als-rewrite-followup branch from cb6d5e6 to 604d429 Compare August 8, 2024 20:45
Qard added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2024

nodejs-github-bot commented Aug 8, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60980/

nodejs-github-bot commented Aug 9, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60990/

Qard added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 9, 2024
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 9, 2024
nodejs-github-bot merged commit 7366808 into nodejs:main Aug 9, 2024

nodejs-github-bot commented Aug 9, 2024

Copy link
Copy Markdown
Collaborator

Landed in 7366808

Qard deleted the als-rewrite-followup branch August 9, 2024 20:17
targos pushed a commit that referenced this pull request Aug 14, 2024
lib: improve async_context_frame structure
b897057 
PR-URL: #54239
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
RafaelGSS mentioned this pull request Aug 19, 2024
Merged
mcollina pushed a commit to mcollina/node that referenced this pull request Dec 12, 2025
lib: improve async_context_frame structure
c7330e6 
PR-URL: nodejs#54239
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Chengzhong Wu <legendecas@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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

legendecas legendecas approved these changes
Flarna Flarna approved these changes

Assignees

No one assigned

Labels

async_local_storage AsyncLocalStorage diag-agenda Issues and PRs to discuss during the meetings of the diagnostics working group. doc Issues and PRs related to the documentations. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL