-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
autocannon tests that will run against the flights routes of the old and new code
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.
Very good work! I've left a couple of notes.
It's impressive how better this is.
test/perf-flights.js
Outdated
connections: 10, | ||
pipelining: 1, | ||
duration: 30 | ||
}, console.log) |
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.
V8 optimizes code as it runs, so it's better to run the test twice, and only print the second result. The results should improve.
Also, you should add https://github.com/mcollina/autocannon#autocannontrackinstance-opts so that it prints a nice progress bar (and output) as it runs.
test/perf-all-flights.js
Outdated
@@ -0,0 +1,19 @@ | |||
'use strict' |
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.
I would move these under a benchmarks/
folder.
Did you run Doctor on old-acmeair? |
Your standard deviation is crazy high so I doubt it is statistically significant. But mathematically it is impossible to say without knowing how many observations the mean and stddev. were sampled from. And as far as I can tell, that information is not provided by the autocannon output. |
Also, am I correct in understanding the only difference is adding the |
@mcollina I have not run doctor on the old one. Do you think its worth running? @AndreasMadsen the PR adds that plugin and the autocannon scripts currently located under /test |
moving autocannon scripts to benchmarks
The approach that autocannon takes is that it samples once per second the number of requests that happened within that second (https://github.com/mcollina/autocannon/blob/master/lib/run.js#L119-L122). So, it's 30 samples. Would you mind to open an issue in https://github.com/mcollina/autocannon on how to better record this? I think this area can definitely be improved. Having a minum of 87 req/s vs a max of 3000 req/s tells that there is something weird happening somewhere. My bet is something database-related. All of this being said, the amount of total requests completed successfully is 59452 vs 4962, which is staggering. |
results from the old code with doctor
|
steps
resultsthese are the results from the second run on autocannon.
old acmeair
|
This PR makes a few changes
The test uses autocannon to request all flight data for a specific airport. This produces two queries. One to lookup the flight segments and the other the flights for those segments.
30 flight segments
174 flights
Using the test for the flights against the API while
clinic doctor
is running produces graphs like below:The current app on this PR doesn't approach the flight query the same as the original. The original was only looking up one flight segment, which then would produce one flight. The autocannon results against the older code looked like this:
The performance increase looks significant with these changes, but I'd like someone to take a look and make sure the results I'm generating are reality.
cc: @jasnell @mcollina
related: #3