Skip to content

Conversation

@legendecas
Copy link
Member

@legendecaslegendecas commented Dec 26, 2019

Refs: nodejs/diagnostics#124

FWIW following is the result of the benchmark:

async_hooks/http-server.js connections=50 asyncHooks="init" benchmarker="autocannon": 31,934.4 async_hooks/http-server.js connections=500 asyncHooks="init" benchmarker="autocannon": 31,830.4 async_hooks/http-server.js connections=50 asyncHooks="before" benchmarker="autocannon": 33,041.6 async_hooks/http-server.js connections=500 asyncHooks="before" benchmarker="autocannon": 30,643.2 async_hooks/http-server.js connections=50 asyncHooks="after" benchmarker="autocannon": 32,300.8 async_hooks/http-server.js connections=500 asyncHooks="after" benchmarker="autocannon": 29,969.6 async_hooks/http-server.js connections=50 asyncHooks="all" benchmarker="autocannon": 33,208 async_hooks/http-server.js connections=500 asyncHooks="all" benchmarker="autocannon": 32,388.8 async_hooks/http-server.js connections=50 asyncHooks="disabled" benchmarker="autocannon": 34,539.2 async_hooks/http-server.js connections=500 asyncHooks="disabled" benchmarker="autocannon": 32,571.2 async_hooks/http-server.js connections=50 asyncHooks="none" benchmarker="autocannon": 34,468.81 async_hooks/http-server.js connections=500 asyncHooks="none" benchmarker="autocannon": 31,824 
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@legendecaslegendecas added the async_hooks Issues and PRs related to the async hooks subsystem. label Dec 26, 2019
@nodejs-github-botnodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Dec 26, 2019
Copy link
Member

@TrottTrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks test/benchmark/test-benchmark-async-hooks.js (which isn't run on every pull request but is run on nightly runs). You'll want to add the asyncHooks and c settings to that test. (Or add asyncHooks only and rename c to n in this benchmark.)

Feel free to dismiss this "request changes" review once the test is updated and passing.

@legendecaslegendecasforce-pushed the benchmark-async-hooks branch 2 times, most recently from 0795d79 to e57bf76CompareJanuary 1, 2020 09:13
@legendecaslegendecasforce-pushed the benchmark-async-hooks branch from e57bf76 to 500b240CompareJanuary 1, 2020 14:22
@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 2, 2020
@legendecas
Copy link
MemberAuthor

Just updated the ordering according to @Trott 's suggestion.

BridgeAR pushed a commit that referenced this pull request Jan 2, 2020
PR-URL: #31100 Refs: nodejs/diagnostics#124 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
@BridgeAR
Copy link
Member

Landed in bf7265f 🎉

@BridgeARBridgeAR closed this Jan 2, 2020
@legendecaslegendecas deleted the benchmark-async-hooks branch January 3, 2020 02:34
BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
PR-URL: #31100 Refs: nodejs/diagnostics#124 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
@BridgeARBridgeAR mentioned this pull request Jan 7, 2020
targos pushed a commit that referenced this pull request Jan 14, 2020
PR-URL: #31100 Refs: nodejs/diagnostics#124 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
@targostargos mentioned this pull request Jan 15, 2020
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #31100 Refs: nodejs/diagnostics#124 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Feb 8, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_hooksIssues and PRs related to the async hooks subsystem.author readyPRs that have at least one approval, no pending requests for changes, and a CI started.benchmarkIssues and PRs related to the benchmark subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@legendecas@BridgeAR@jasnell@Trott@nodejs-github-bot