-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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.
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
benchmark: chunky http client benchmark variation and server #228
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
"use strict"; | ||
// test HTTP throughput in fragmented header case | ||
var common = require('../common.js'); | ||
var net = require('net'); | ||
var test = require('../../test/common.js'); | ||
|
||
var bench = common.createBenchmark(main, { | ||
// len: [1, 4, 8, 16, 32, 64, 128], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: don't leave commented out code in. |
||
len: [1], | ||
num: [5, 50, 500, 2000], | ||
type: ['send'], | ||
} | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: |
||
|
||
|
||
function main(conf) { | ||
var len; | ||
var num; | ||
var type; | ||
num = +conf.num; | ||
len = +conf.len; | ||
type = conf.type; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: declaration and assignment normally go on the same line. You can use let or const if you want. |
||
var todo = []; | ||
var headers = []; | ||
// Chose 7 because 9 showed "Connection error" / "Connection closed" | ||
// An odd number could result in a better length dispersion. | ||
for (var i = 7; i <= 7*7*7; i *= 7) | ||
headers.push(Array(i + 1).join('o')); | ||
|
||
function WriteWithCRLF(line) { | ||
todo.push(line + '\r\n'); | ||
} | ||
|
||
function WriteHTTPHeaders(channel, has_keep_alive, extra_header_count) { | ||
todo = [] | ||
WriteWithCRLF('GET / HTTP/1.1'); | ||
WriteWithCRLF('Host: localhost'); | ||
WriteWithCRLF('Connection: keep-alive'); | ||
WriteWithCRLF('Accept: text/html,application/xhtml+xml,' + | ||
'application/xml;q=0.9,image/webp,*/*;q=0.8'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: string should line up with the one above it. |
||
WriteWithCRLF('User-Agent: Mozilla/5.0 (X11; Linux x86_64) ' + | ||
'AppleWebKit/537.36 (KHTML, like Gecko) ' + | ||
'Chrome/39.0.2171.71 Safari/537.36'); | ||
WriteWithCRLF('Accept-Encoding: gzip, deflate, sdch'); | ||
WriteWithCRLF('Accept-Language: en-US,en;q=0.8'); | ||
for (var i = 0; i < extra_header_count; i++) { | ||
// Utilize first three powers of a small integer for an odd cycle and | ||
// because the fourth power of some integers overloads the server. | ||
WriteWithCRLF('X-Header-' + i + ': ' + headers[i%3]); | ||
} | ||
WriteWithCRLF(''); | ||
todo = todo.join(''); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can replace WriteWithCRLF() with todo.push(), then do |
||
// Using odd numbers in many places may increase length coverage. | ||
var chunksize = 37; | ||
for (i = 0; i < todo.length; i+=chunksize) { | ||
var cur = todo.slice(i,i+chunksize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: spaces before and after operators (here and elsewhere.) |
||
channel.write(cur); | ||
} | ||
} | ||
|
||
var success = 0, failure = 0, min = 10, size = 0; | ||
var mod = 317, mult = 17, add = 11; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: prefer one declaration per line. |
||
var count = 0; | ||
var PIPE = test.PIPE; | ||
var socket = net.connect(PIPE, function() { | ||
bench.start(); | ||
WriteHTTPHeaders(socket, 1, +conf.len); | ||
socket.setEncoding('utf8') | ||
socket.on('data', function(d) { | ||
var did = false; | ||
var pattern = 'HTTP/1.1 200 OK\r\n'; | ||
if ((d.length === pattern.length && d === pattern) || | ||
(d.length > pattern.length && | ||
d.slice(0, pattern.length) === pattern)) { | ||
success += 1; | ||
did = true; | ||
} else { | ||
pattern = 'HTTP/1.1 ' | ||
if ((d.length === pattern.length && d === pattern) || | ||
(d.length > pattern.length && | ||
d.slice(0, pattern.length) === pattern)) { | ||
failure += 1; | ||
did = true; | ||
} | ||
} | ||
size = (size * mult + add) % mod; | ||
if (did) { | ||
count += 1; | ||
if (count === num) { | ||
bench.end(count); | ||
} else { | ||
WriteHTTPHeaders(socket, 1, min + size); | ||
} | ||
} | ||
}); | ||
socket.on('close', function() { | ||
console.log('Connection closed'); | ||
}); | ||
|
||
socket.on('error', function() { | ||
console.log('Connection error'); | ||
}); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: indentation seems off here. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
"use strict"; | ||
var path = require('path'); | ||
var http = require('http'); | ||
var fs = require('fs'); | ||
var spawn = require('child_process').spawn; | ||
var common = require('../common.js') | ||
var test = require('../../test/common.js') | ||
var pep = path.dirname(process.argv[1])+'/chunky_http_client.js'; | ||
var PIPE = test.PIPE; | ||
|
||
var server; | ||
|
||
fs.unlinkSync(PIPE); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could throw with try {
fs.unlinkSync(PIPE);
} catch (e) { } |
||
server = http.createServer(function(req, res) { | ||
res.writeHead(200, { 'content-type': 'text/plain', | ||
'content-length': '2' }); | ||
res.end('ok'); | ||
}); | ||
|
||
server.on('error', function(err) { | ||
console.log("Error:"); | ||
console.log(err); | ||
}); | ||
|
||
try { | ||
server.listen(PIPE, 'localhost'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'localhost' is not needed and you may have a race condition here. The child process should be started from inside the 'listening' event listener. |
||
} catch(e) { | ||
console.log("oops!"); | ||
console.log(e); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused by your use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to Trevor and BTW thanks for the guidance and additional comments they are quite helpful. Even though we do an unlinkSync(PIPE) above, it seems to be no guarantee against another mistaken process interfering accidentally. I seem to remember that it is possible to bind to a port with an AF_INET address exclusively or not a la SO_REUSEADDR / etc and I had assumed there was some similar risks with AF_UNIX family that another process might accidentally do. Please let me know if you believe otherwise (e.g. call cannot ever fail) and I would like to find out more as well and simplify this code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the socket path is taken, then there's nothing you can do anyway. Best to just let the script fail. |
||
|
||
var child = spawn(process.execPath, [pep], { }); | ||
child.on('error', function(err) { | ||
console.log('spawn error.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For all errors, please use |
||
console.log(err); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're not going to handle them, you should let 'error' events bubble up. |
||
|
||
child.stdout.on('data', function (data) { | ||
process.stdout.write(data); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
child.on('exit', function (exitCode) { | ||
console.log("Child exited with code: " + exitCode); | ||
process.exit(0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally, tests and benchmarks terminate without calling process.exit(). process.exit() sometimes hides bugs in the script. |
||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: the convention is to use single quotes. There is usually a blank line after 'use strict'.