Skip to content

benchmark: cut down http benchmark run time - #18379

Closed
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:cut-http-bench
Closed

benchmark: cut down http benchmark run time#18379
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:cut-http-bench

Conversation

@joyeecheung

Copy link
Copy Markdown
Member

This split the http/simple.js benchmark so the header settings can be benchmarked differently to reduce the time spent on this benchmark. IIUC other parameters do not have an impact on the results, see the OP of #10558

Also cut the default duration to 5 seconds, on the benchmark CI we have about 20k req/sec for most types of scenarios so the results from 100k reqs should be representative enough.

cc @mscdex

Refs: #18003 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

benchmark

@nodejs-github-botnodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem. labels Jan 25, 2018
@joyeecheung

Copy link
Copy Markdown
MemberAuthor

After #18373 lands we can see how the accuracy changes with the new parameters.

@joyeecheung

Copy link
Copy Markdown
MemberAuthor

@joyeecheung

Copy link
Copy Markdown
MemberAuthor

CI failures are unrelated

@joyeecheung

Copy link
Copy Markdown
MemberAuthor

@joyeecheung

joyeecheung commented Jan 30, 2018

Copy link
Copy Markdown
MemberAuthor

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 31, 2018
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Feb 1, 2018
PR-URL: nodejs#18379
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR

Copy link
Copy Markdown
Member

Landed in 54b3a68

@BridgeARBridgeAR closed this Feb 1, 2018
@addaleaxaddaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 4, 2018
MylesBorins pushed a commit that referenced this pull request Feb 20, 2018
PR-URL: #18379
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
PR-URL: #18379
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
PR-URL: #18379
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Feb 21, 2018
@MylesBorins

Copy link
Copy Markdown
Contributor

Should this be backported to v8.x-staging or v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#18379
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmarkIssues and PRs related to the benchmark subsystem.httpIssues or PRs related to the http subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@joyeecheung@BridgeAR@MylesBorins@jasnell@cjihrig@maclover7@addaleax@nodejs-github-bot