Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

benchmark: fix http/simple.js benchmark #17583

Closed
wants to merge 1 commit into from
Closed

benchmark: fix http/simple.js benchmark #17583

wants to merge 1 commit into from

Conversation

apapirovski
Copy link
Member

autocannon appears to have trouble recognizing URLs that contain false or true within them. Use 0 or 1 instead to represent the same. Prior to this fix, autocannon would just print out its help instead of running the actual benchmark.

I'm guessing this also needs fixing in autocannon or whatever its using to parse arguments but for now I would like to get our http benchmark working again.

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

benchmark

@apapirovski
benchmark: fix http/simple.js benchmark
02c9653 
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.
@apapirovski apapirovski added benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem. labels Dec 9, 2017
@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem. labels Dec 9, 2017
@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 11, 2017
@addaleax
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-commit/14751/

@apapirovski
Copy link
Member Author

Landed in 4df7c19

Thanks for the reviews, everyone!

@apapirovski apapirovski deleted the fix-http-simple-benchmark branch December 11, 2017 23:18
apapirovski added a commit that referenced this pull request Dec 11, 2017
@apapirovski
benchmark: fix http/simple.js benchmark
4df7c19 
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

PR-URL: #17583
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@apapirovski @MylesBorins
benchmark: fix http/simple.js benchmark
3c918c7 
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

PR-URL: #17583
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@apapirovski @MylesBorins
benchmark: fix http/simple.js benchmark
cbd0be5 
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

PR-URL: #17583
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 13, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@apapirovski @gibfahn
benchmark: fix http/simple.js benchmark
e93a6da 
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

PR-URL: #17583
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@apapirovski @gibfahn
benchmark: fix http/simple.js benchmark
13b6e96 
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

PR-URL: #17583
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Merged
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@apapirovski @gibfahn
benchmark: fix http/simple.js benchmark
ce91a38 
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

PR-URL: #17583
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@apapirovski @addaleax @cjihrig @BridgeAR @gibfahn @nodejs-github-bot