Skip to content

Conversation

legendecas
Copy link
Member

@legendecas legendecas 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

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

@Trott Trott 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.

@legendecas legendecas force-pushed the benchmark-async-hooks branch 2 times, most recently from 0795d79 to e57bf76 Compare January 1, 2020 09:13
@legendecas legendecas force-pushed the benchmark-async-hooks branch from e57bf76 to 500b240 Compare January 1, 2020 14:22
@BridgeAR BridgeAR 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
Member Author

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

BridgeAR commented Jan 2, 2020

Landed in bf7265f 🎉

@BridgeAR BridgeAR closed this Jan 2, 2020
@legendecas legendecas 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]>
@BridgeAR BridgeAR 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]>
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]>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants