From 3c43e00d8a05310d014f08a1d7d4cc73856561f4 Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Fri, 28 Jul 2017 17:08:20 -0400 Subject: [PATCH 01/14] Timing (#1) * Adding timing-sensitive load test option in h2load. * more checks added for parameters * A worker thread can control its clients' warmup and main duration. * Changed warmup to an enum variable. * removed unnecessary call to ev_timer_stop * assertion is done before starting main measurement phase * phase variable is implemented only inside the Worker class * enum to enum class * else indentation corrected * check added for timing-based test when duration CB is called explicitly * New argument is introduced for timing-based benchmarking. * styling corrections * duration watcher initialization is pushed back into warmup timeout * Warmup and Duration timer is moved to Worker instead of clients. Now both timers and phase belongs to the Workers. * some client functions are modified to return if it's not main_duration phase. client is not destructed but sessions are terminated * outputs are adjusted for thread. * Needed to check if a session exist before terminating * formatting * more formatting * formatting --- src/h2load.cc | 305 ++++++++++++++++++++++++++++++++++++++++---------- src/h2load.h | 23 ++++ 2 files changed, 269 insertions(+), 59 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 492d8d9f..8840fca8 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -90,6 +90,8 @@ Config::Config() connection_window_bits(30), rate(0), rate_period(1.0), + duration(0.0), + warm_up_time(0.0), conn_active_timeout(0.), conn_inactivity_timeout(0.), no_tls_proto(PROTO_HTTP2), @@ -118,6 +120,7 @@ Config::~Config() { } bool Config::is_rate_mode() const { return (this->rate != 0); } +bool Config::is_timing_based_mode() const { return (this->duration > 0); } bool Config::has_base_uri() const { return (!this->base_uri.empty()); } Config config; @@ -225,6 +228,11 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { auto nclients_per_second = worker->rate; auto conns_remaining = worker->nclients - worker->nconns_made; auto nclients = std::min(nclients_per_second, conns_remaining); + if (worker->config->is_timing_based_mode() > 0) { + worker->current_phase = Phase::INITIAL_IDLE; + } else { + worker->current_phase = Phase::MAIN_DURATION; + } for (size_t i = 0; i < nclients; ++i) { auto req_todo = worker->nreqs_per_client; @@ -236,6 +244,7 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { make_unique(worker->next_client_id++, worker, req_todo); ++worker->nconns_made; + worker->clients.push_back(client.get()); if (client->connect() != 0) { std::cerr << "client could not connect to host" << std::endl; @@ -251,6 +260,60 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { } } // namespace +namespace { +// Called when the duration for infinite number of requests are over +void duration_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { + auto worker = static_cast(w->data); + + for (auto client: worker->clients) { + client->req_todo = client->req_done; // there was no finite "req_todo" + worker->stats.req_todo += client->req_todo; + client->req_inflight = 0; + client->req_left = 0; + } + + worker->current_phase = Phase::DURATION_OVER; + + std::cout << "Main benchmark duration is over for thread #" << worker->id + << ". Stopping all clients." << std::endl; + worker->stop_all_clients(); + std::cout << "Stopped all clients for thread #" << worker->id << std::endl; +} +} // namespace + +namespace { +// Called when the warmup duration for infinite number of requests are over +void warmup_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { + auto worker = static_cast(w->data); + + std::cout << "Warm-up phase is over for thread #" << worker->id << "." + << std::endl; + std::cout << "Main benchmark duration is started for thread #" << worker->id + << "." << std::endl; + assert(worker->stats.req_started == 0); + assert(worker->stats.req_done == 0); + + for (auto client : worker->clients) { + assert(client->req_todo == 0); + assert(client->req_left == 1); + assert(client->req_inflight == 0); + assert(client->req_started == 0); + assert(client->req_done == 0); + + client->record_client_start_time(); + client->clear_connect_times(); + client->record_connect_start_time(); + } + + worker->current_phase = Phase::MAIN_DURATION; + + ev_timer_init(&worker->duration_watcher, duration_timeout_cb, + worker->config->duration, 0.); + + ev_timer_start(worker->loop, &worker->duration_watcher); +} +} // namespace + namespace { // Called when an a connection has been inactive for a set period of time // or a fixed amount of time after all requests have been made on a @@ -336,6 +399,11 @@ Client::Client(uint32_t id, Worker *worker, size_t req_todo) fd(-1), new_connection_requested(false), final(false) { + if (req_todo == 0) { // this means infinite number of requests are to be made + // This ensures that number of requests are unbounded + // Just a positive number is fine, we chose the first positive number + req_left = 1; + } ev_io_init(&wev, writecb, 0, EV_WRITE); ev_io_init(&rev, readcb, 0, EV_READ); @@ -407,9 +475,17 @@ int Client::make_socket(addrinfo *addr) { int Client::connect() { int rv; - record_client_start_time(); - clear_connect_times(); - record_connect_start_time(); + if (!worker->config->is_timing_based_mode() + || worker->current_phase == Phase::MAIN_DURATION) { + record_client_start_time(); + clear_connect_times(); + record_connect_start_time(); + } else if (worker->current_phase == Phase::INITIAL_IDLE) { + worker->current_phase = Phase::WARM_UP; + std::cout << "Warm-up started for thread #" << worker->id + << "." << std::endl; + ev_timer_start(worker->loop, &worker->warmup_watcher); + } if (worker->config->conn_inactivity_timeout > 0.) { ev_timer_again(worker->loop, &conn_inactivity_watcher); @@ -467,13 +543,17 @@ int Client::try_again_or_fail() { if (new_connection_requested) { new_connection_requested = false; - if (req_left) { - // At the moment, we don't have a facility to re-start request - // already in in-flight. Make them fail. - worker->stats.req_failed += req_inflight; - worker->stats.req_error += req_inflight; - req_inflight = 0; + if (req_left) { + + if (worker->current_phase == Phase::MAIN_DURATION) { + // At the moment, we don't have a facility to re-start request + // already in in-flight. Make them fail. + worker->stats.req_failed += req_inflight; + worker->stats.req_error += req_inflight; + + req_inflight = 0; + } // Keep using current address if (connect() == 0) { @@ -529,11 +609,16 @@ int Client::submit_request() { return -1; } + if (worker->current_phase != Phase::MAIN_DURATION) { + return 0; + } + ++worker->stats.req_started; - --req_left; ++req_started; ++req_inflight; - + if (!worker->config->is_timing_based_mode()) { + --req_left; + } // if an active timeout is set and this is the last request to be submitted // on this connection, start the active timeout. if (worker->config->conn_active_timeout > 0. && req_left == 0) { @@ -544,6 +629,10 @@ int Client::submit_request() { } void Client::process_timedout_streams() { + if (worker->current_phase != Phase::MAIN_DURATION) { + return; + } + for (auto &p : streams) { auto &req_stat = p.second.req_stat; if (!req_stat.completed) { @@ -557,6 +646,10 @@ void Client::process_timedout_streams() { } void Client::process_abandoned_streams() { + if (worker->current_phase != Phase::MAIN_DURATION) { + return; + } + auto req_abandoned = req_inflight + req_left; worker->stats.req_failed += req_abandoned; @@ -567,6 +660,10 @@ void Client::process_abandoned_streams() { } void Client::process_request_failure() { + if (worker->current_phase != Phase::MAIN_DURATION) { + return; + } + worker->stats.req_failed += req_left; worker->stats.req_error += req_left; @@ -575,6 +672,7 @@ void Client::process_request_failure() { if (req_inflight == 0) { terminate_session(); } + std::cout << "Process Request Failure:" << worker->stats.req_failed << std::endl; } namespace { @@ -648,6 +746,9 @@ void Client::on_request(int32_t stream_id) { streams[stream_id] = Stream(); } void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen) { + if (worker->current_phase != Phase::MAIN_DURATION) { + return; + } auto itr = streams.find(stream_id); if (itr == std::end(streams)) { return; @@ -668,7 +769,7 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, break; } } - + if (status >= 200 && status < 300) { ++worker->stats.status[2]; stream.status_success = 1; @@ -685,6 +786,10 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, } void Client::on_status_code(int32_t stream_id, uint16_t status) { + if (worker->current_phase != Phase::MAIN_DURATION) { + return; + } + auto itr = streams.find(stream_id); if (itr == std::end(streams)) { return; @@ -706,40 +811,42 @@ void Client::on_status_code(int32_t stream_id, uint16_t status) { } void Client::on_stream_close(int32_t stream_id, bool success, bool final) { - ++req_done; - --req_inflight; + if (worker->current_phase == Phase::MAIN_DURATION) { + ++req_done; + if (req_inflight > 0) { + --req_inflight; + } + auto req_stat = get_req_stat(stream_id); + if (!req_stat) { + return; + } - auto req_stat = get_req_stat(stream_id); - if (!req_stat) { - return; - } + req_stat->stream_close_time = std::chrono::steady_clock::now(); + if (success) { + req_stat->completed = true; + ++worker->stats.req_success; + ++cstat.req_success; + + if (streams[stream_id].status_success == 1) { + ++worker->stats.req_status_success; + } else { + ++worker->stats.req_failed; + } - req_stat->stream_close_time = std::chrono::steady_clock::now(); - if (success) { - req_stat->completed = true; - ++worker->stats.req_success; - ++cstat.req_success; + if (sampling_should_pick(worker->request_times_smp)) { + sampling_advance_point(worker->request_times_smp); + worker->sample_req_stat(req_stat); + } - if (streams[stream_id].status_success == 1) { - ++worker->stats.req_status_success; + // Count up in successful cases only + ++worker->request_times_smp.n; } else { ++worker->stats.req_failed; + ++worker->stats.req_error; } - - if (sampling_should_pick(worker->request_times_smp)) { - sampling_advance_point(worker->request_times_smp); - worker->sample_req_stat(req_stat); - } - - // Count up in successful cases only - ++worker->request_times_smp.n; - } else { - ++worker->stats.req_failed; - ++worker->stats.req_error; + ++worker->stats.req_done; } - ++worker->stats.req_done; - worker->report_progress(); streams.erase(stream_id); if (req_left == 0 && req_inflight == 0) { @@ -906,7 +1013,9 @@ int Client::on_read(const uint8_t *data, size_t len) { if (rv != 0) { return -1; } - worker->stats.bytes_total += len; + if (worker->current_phase == Phase::MAIN_DURATION) { + worker->stats.bytes_total += len; + } signal_write(); return 0; } @@ -1136,7 +1245,7 @@ void Client::record_client_start_time() { if (recorded(cstat.client_start_time)) { return; } - + cstat.client_start_time = std::chrono::steady_clock::now(); } @@ -1176,12 +1285,13 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, rate(rate), max_samples(max_samples), next_client_id(0) { - if (!config->is_rate_mode()) { + if (!config->is_rate_mode() && !config->is_timing_based_mode()) { progress_interval = std::max(static_cast(1), req_todo / 10); } else { progress_interval = std::max(static_cast(1), nclients / 10); } + // Below timeout is not needed in case of timing-based benchmarking // create timer that will go off every rate_period ev_timer_init(&timeout_watcher, rate_period_timeout_w_cb, 0., config->rate_period); @@ -1192,6 +1302,14 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, sampling_init(request_times_smp, req_todo, max_samples); sampling_init(client_smp, nclients, max_samples); + + if (config->is_timing_based_mode()) { + duration_watcher.data = this; + + ev_timer_init(&warmup_watcher, warmup_timeout_cb, + config->warm_up_time, 0.); + warmup_watcher.data = this; + } } Worker::~Worker() { @@ -1199,14 +1317,23 @@ Worker::~Worker() { ev_loop_destroy(loop); } +void Worker::stop_all_clients() { + for (auto client : clients) { + if (client->session) { + client->terminate_session(); + } + } +} + void Worker::run() { - if (!config->is_rate_mode()) { + if (!config->is_rate_mode() && !config->is_timing_based_mode()) { for (size_t i = 0; i < nclients; ++i) { auto req_todo = nreqs_per_client; if (nreqs_rem > 0) { ++req_todo; --nreqs_rem; } + auto client = make_unique(next_client_id++, this, req_todo); if (client->connect() != 0) { std::cerr << "client could not connect to host" << std::endl; @@ -1215,11 +1342,14 @@ void Worker::run() { client.release(); } } - } else { + } else if (config->is_rate_mode()){ ev_timer_again(loop, &timeout_watcher); // call callback so that we don't waste the first rate_period rate_period_timeout_w_cb(loop, &timeout_watcher, 0); + } else { + // call the callback to start for one single time + rate_period_timeout_w_cb(loop, &timeout_watcher, 0); } ev_run(loop, 0); } @@ -1235,7 +1365,8 @@ void Worker::sample_client_stat(ClientStat *cstat) { } void Worker::report_progress() { - if (id != 0 || config->is_rate_mode() || stats.req_done % progress_interval) { + if (id != 0 || config->is_rate_mode() || stats.req_done % progress_interval + || config->is_timing_based_mode()) { return; } @@ -1581,12 +1712,27 @@ std::unique_ptr create_worker(uint32_t id, SSL_CTX *ssl_ctx, << util::duration_str(config.rate_period) << " "; } - std::cout << "spawning thread #" << id << ": " << nclients - << " total client(s). " << rate_report.str() << nreqs - << " total requests" << std::endl; + if (config.is_timing_based_mode()) { + std::cout << "spawning thread #" << id << ": " << nclients + << " total client(s). Timing-based test with " + << config.warm_up_time << "s of warm-up time and " + << config.duration << "s of main duration for measurements." + << std::endl; + } else { + std::cout << "spawning thread #" << id << ": " << nclients + << " total client(s). " << rate_report.str() << nreqs + << " total requests" << std::endl; + } - return make_unique(id, ssl_ctx, nreqs, nclients, rate, max_samples, - &config); + if (config.is_rate_mode()) { + return make_unique(id, ssl_ctx, nreqs, nclients, rate, max_samples, + &config); + } else { + // Here rate is same as client because the rate_timeout callback + // will be called only once + return make_unique(id, ssl_ctx, nreqs, nclients, nclients, max_samples, + &config); + } } } // namespace @@ -1737,6 +1883,13 @@ Options: length of the period in time. This option is ignored if the rate option is not used. The default value for this option is 1s. + -D, --duration= + Specifies the main duration for the measurements in case + of timing-based benchmarking. + --warm-up-time= + Specifies the time period before starting the actual + measurements, in case of timing-based benchmarking. + Needs to provided along with -D option. -T, --connection-active-timeout= Specifies the maximum time that h2load is willing to keep a connection open, regardless of the activity on @@ -1850,6 +2003,7 @@ int main(int argc, char **argv) { {"rate", required_argument, nullptr, 'r'}, {"connection-active-timeout", required_argument, nullptr, 'T'}, {"connection-inactivity-timeout", required_argument, nullptr, 'N'}, + {"duration", required_argument, &flag, 'D'}, {"timing-script-file", required_argument, &flag, 3}, {"base-uri", required_argument, nullptr, 'B'}, {"npn-list", required_argument, &flag, 4}, @@ -1857,10 +2011,11 @@ int main(int argc, char **argv) { {"h1", no_argument, &flag, 6}, {"header-table-size", required_argument, &flag, 7}, {"encoder-header-table-size", required_argument, &flag, 8}, + {"warm-up-time", required_argument, &flag, 9}, {nullptr, 0, nullptr, 0}}; int option_index = 0; auto c = - getopt_long(argc, argv, "hvW:c:d:m:n:p:t:w:H:i:r:T:N:B:", long_options, + getopt_long(argc, argv, "hvW:c:d:m:n:p:t:w:H:i:r:T:N:D:B:", long_options, &option_index); if (c == -1) { break; @@ -2016,6 +2171,14 @@ int main(int argc, char **argv) { config.base_uri = arg.str(); break; } + case 'D': + config.duration = strtoul(optarg, nullptr, 10); + if (config.duration == 0) { + std::cerr << "-D: the main duration for timing-based benchmarking " + << "must be positive." << std::endl; + exit(EXIT_FAILURE); + } + break; case 'v': config.verbose = true; break; @@ -2072,6 +2235,14 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } break; + case 9: + // --warm-up-time + config.warm_up_time = util::parse_duration_with_unit(optarg); + if (!std::isfinite(config.warm_up_time)) { + std::cerr << "--warm-up-time: value error " << optarg << std::endl; + exit(EXIT_FAILURE); + } + break; } break; default: @@ -2157,8 +2328,9 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } - if (config.nreqs == 0) { - std::cerr << "-n: the number of requests must be strictly greater than 0." + if (config.nreqs == 0 && !config.is_timing_based_mode()) { + std::cerr << "-n: the number of requests must be strictly greater than 0," + << "timing-based test is not being run." << std::endl; exit(EXIT_FAILURE); } @@ -2182,7 +2354,7 @@ int main(int argc, char **argv) { // With timing script, we don't distribute config.nreqs to each // client or thread. - if (!config.timing_script && config.nreqs < config.nclients) { + if (!config.timing_script && config.nreqs < config.nclients && !config.is_timing_based_mode()) { std::cerr << "-n, -c: the number of requests must be greater than or " << "equal to the clients." << std::endl; exit(EXIT_FAILURE); @@ -2190,11 +2362,20 @@ int main(int argc, char **argv) { if (config.nclients < config.nthreads) { std::cerr << "-c, -t: the number of clients must be greater than or equal " - "to the number of threads." + << "to the number of threads." << std::endl; exit(EXIT_FAILURE); } + if (config.is_timing_based_mode()) { + if (config.nreqs != 0) { + std::cerr << "-n: the number of requests needs to be zero (0) for timing-" + << "based test. Default value is 1." + << std::endl; + exit(EXIT_FAILURE); + } + } + if (config.is_rate_mode()) { if (config.rate < config.nthreads) { std::cerr << "-r, -t: the connection rate must be greater than or equal " @@ -2528,7 +2709,7 @@ int main(int argc, char **argv) { // Requests which have not been issued due to connection errors, are // counted towards req_failed and req_error. auto req_not_issued = - stats.req_todo - stats.req_status_success - stats.req_failed; + (stats.req_todo - stats.req_status_success - stats.req_failed); stats.req_failed += req_not_issued; stats.req_error += req_not_issued; @@ -2539,10 +2720,16 @@ int main(int argc, char **argv) { double rps = 0; int64_t bps = 0; if (duration.count() > 0) { - auto secd = std::chrono::duration_cast< - std::chrono::duration>(duration); - rps = stats.req_success / secd.count(); - bps = stats.bytes_total / secd.count(); + if (config.is_timing_based_mode()) { + // we only want to consider the main duration if warm-up is given + rps = stats.req_success / config.duration; + bps = stats.bytes_total / config.duration; + } else { + auto secd = std::chrono::duration_cast< + std::chrono::duration>(duration); + rps = stats.req_success / secd.count(); + bps = stats.bytes_total / secd.count(); + } } double header_space_savings = 0.; diff --git a/src/h2load.h b/src/h2load.h index db50472d..077f5752 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -85,6 +85,10 @@ struct Config { // rate at which connections should be made size_t rate; ev_tstamp rate_period; + // amount of time for main measurements in timing-based test + ev_tstamp duration; + // amount of time to wait before starting measurements in timing-based test + ev_tstamp warm_up_time; // amount of time to wait for activity on a given connection ev_tstamp conn_active_timeout; // amount of time to wait after the last request is made on a connection @@ -118,6 +122,7 @@ struct Config { ~Config(); bool is_rate_mode() const; + bool is_timing_based_mode() const; bool has_base_uri() const; }; @@ -215,6 +220,15 @@ struct Stats { enum ClientState { CLIENT_IDLE, CLIENT_CONNECTED }; +// This type tells whether the client is in warmup phase or not or is over +enum class Phase { + INITIAL_IDLE, // Initial idle state before warm-up phase + WARM_UP, // Warm up phase when no measurements are done + MAIN_DURATION, // Main measurement phase; if timing-based + // test is not run, this is the default phase + DURATION_OVER // This phase occurs after the measurements are over +}; + struct Client; // We use systematic sampling method @@ -253,6 +267,13 @@ struct Worker { ev_timer timeout_watcher; // The next client ID this worker assigns uint32_t next_client_id; + // Keeps track of the current phase (for timing-based experiment) for the worker + Phase current_phase; + // We need to keep track of the clients in order to stop them when needed + std::vector clients; + // This is only active when there is not a bounded number of requests specified + ev_timer duration_watcher; + ev_timer warmup_watcher; Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t nreq_todo, size_t nclients, size_t rate, size_t max_samples, Config *config); @@ -263,6 +284,8 @@ struct Worker { void sample_client_stat(ClientStat *cstat); void report_progress(); void report_rate_progress(); + // This function calls the destructors of all the clients. + void stop_all_clients(); }; struct Stream { From 5f3c541c4cc569bb7af09176a8286d068490a1c1 Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Fri, 28 Jul 2017 17:31:13 -0400 Subject: [PATCH 02/14] enabled --duration option. --- src/h2load.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/h2load.cc b/src/h2load.cc index 8840fca8..7579edde 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -2003,7 +2003,7 @@ int main(int argc, char **argv) { {"rate", required_argument, nullptr, 'r'}, {"connection-active-timeout", required_argument, nullptr, 'T'}, {"connection-inactivity-timeout", required_argument, nullptr, 'N'}, - {"duration", required_argument, &flag, 'D'}, + {"duration", required_argument, nullptr, 'D'}, {"timing-script-file", required_argument, &flag, 3}, {"base-uri", required_argument, nullptr, 'B'}, {"npn-list", required_argument, &flag, 4}, From e85698e13182a167776026ee55a58810f0739aec Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Tue, 1 Aug 2017 17:45:18 -0400 Subject: [PATCH 03/14] MAIN_DURATION is initiliazed in Worker constructor, MAIN_DURATION check is removed from two functions because those functions are needed in warm-up phase as well. --- src/h2load.cc | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 7579edde..bb2324c9 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -228,11 +228,6 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { auto nclients_per_second = worker->rate; auto conns_remaining = worker->nclients - worker->nconns_made; auto nclients = std::min(nclients_per_second, conns_remaining); - if (worker->config->is_timing_based_mode() > 0) { - worker->current_phase = Phase::INITIAL_IDLE; - } else { - worker->current_phase = Phase::MAIN_DURATION; - } for (size_t i = 0; i < nclients; ++i) { auto req_todo = worker->nreqs_per_client; @@ -746,9 +741,6 @@ void Client::on_request(int32_t stream_id) { streams[stream_id] = Stream(); } void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen) { - if (worker->current_phase != Phase::MAIN_DURATION) { - return; - } auto itr = streams.find(stream_id); if (itr == std::end(streams)) { return; @@ -786,9 +778,6 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, } void Client::on_status_code(int32_t stream_id, uint16_t status) { - if (worker->current_phase != Phase::MAIN_DURATION) { - return; - } auto itr = streams.find(stream_id); if (itr == std::end(streams)) { @@ -812,7 +801,6 @@ void Client::on_status_code(int32_t stream_id, uint16_t status) { void Client::on_stream_close(int32_t stream_id, bool success, bool final) { if (worker->current_phase == Phase::MAIN_DURATION) { - ++req_done; if (req_inflight > 0) { --req_inflight; } @@ -845,6 +833,7 @@ void Client::on_stream_close(int32_t stream_id, bool success, bool final) { ++worker->stats.req_error; } ++worker->stats.req_done; + ++req_done; } worker->report_progress(); @@ -1291,6 +1280,12 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, progress_interval = std::max(static_cast(1), nclients / 10); } + if (config->is_timing_based_mode() > 0) { + current_phase = Phase::INITIAL_IDLE; + } else { + current_phase = Phase::MAIN_DURATION; + } + // Below timeout is not needed in case of timing-based benchmarking // create timer that will go off every rate_period ev_timer_init(&timeout_watcher, rate_period_timeout_w_cb, 0., From 566cee8fe7b8168a22bff666fc14feb57a85f4b9 Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Tue, 1 Aug 2017 17:45:52 -0400 Subject: [PATCH 04/14] MAIN_DURATION is initiliazed in Worker constructor, MAIN_DURATION check is removed from two functions because those functions are needed in warm-up phase as well. --- src/h2load.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/h2load.cc b/src/h2load.cc index bb2324c9..db0de077 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -778,7 +778,6 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, } void Client::on_status_code(int32_t stream_id, uint16_t status) { - auto itr = streams.find(stream_id); if (itr == std::end(streams)) { return; From 0836a514082281b2d57785e555ddf3cf6d65d5db Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Tue, 1 Aug 2017 18:29:00 -0400 Subject: [PATCH 05/14] Handling requests starting in warm-up phase and ending in MAIN_DURATION --- src/h2load.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/h2load.cc b/src/h2load.cc index db0de077..c48e61d2 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -746,6 +746,15 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, return; } auto &stream = (*itr).second; + + if (worker->current_phase != Phase::MAIN_DURATION) { + // If the stream is for warm-up phase, then mark as a success + // But we do not update the count for 2xx, 3xx, etc status codes + // Same has been done in on_status_code function + stream.status_success = 1; + return; + } + if (stream.status_success == -1 && namelen == 7 && util::streq_l(":status", name, namelen)) { int status = 0; @@ -784,6 +793,11 @@ void Client::on_status_code(int32_t stream_id, uint16_t status) { } auto &stream = (*itr).second; + if (worker->current_phase != Phase::MAIN_DURATION) { + stream.status_success = 1; + return; + } + if (status >= 200 && status < 300) { ++worker->stats.status[2]; stream.status_success = 1; @@ -817,8 +831,10 @@ void Client::on_stream_close(int32_t stream_id, bool success, bool final) { if (streams[stream_id].status_success == 1) { ++worker->stats.req_status_success; } else { + // These requests are initialized and their statuses are changed ++worker->stats.req_failed; } + // If statuses are not changed from -1, that means if (sampling_should_pick(worker->request_times_smp)) { sampling_advance_point(worker->request_times_smp); From d068a297981555def27877f27e59e903ace7b11d Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Tue, 1 Aug 2017 19:51:47 -0400 Subject: [PATCH 06/14] removed unnecessary code --- src/h2load.cc | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index c48e61d2..2f9a7fa0 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -746,7 +746,7 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, return; } auto &stream = (*itr).second; - + if (worker->current_phase != Phase::MAIN_DURATION) { // If the stream is for warm-up phase, then mark as a success // But we do not update the count for 2xx, 3xx, etc status codes @@ -754,7 +754,7 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, stream.status_success = 1; return; } - + if (stream.status_success == -1 && namelen == 7 && util::streq_l(":status", name, namelen)) { int status = 0; @@ -827,14 +827,12 @@ void Client::on_stream_close(int32_t stream_id, bool success, bool final) { req_stat->completed = true; ++worker->stats.req_success; ++cstat.req_success; - + if (streams[stream_id].status_success == 1) { ++worker->stats.req_status_success; } else { - // These requests are initialized and their statuses are changed ++worker->stats.req_failed; } - // If statuses are not changed from -1, that means if (sampling_should_pick(worker->request_times_smp)) { sampling_advance_point(worker->request_times_smp); @@ -1295,12 +1293,6 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, progress_interval = std::max(static_cast(1), nclients / 10); } - if (config->is_timing_based_mode() > 0) { - current_phase = Phase::INITIAL_IDLE; - } else { - current_phase = Phase::MAIN_DURATION; - } - // Below timeout is not needed in case of timing-based benchmarking // create timer that will go off every rate_period ev_timer_init(&timeout_watcher, rate_period_timeout_w_cb, 0., @@ -1319,6 +1311,9 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, ev_timer_init(&warmup_watcher, warmup_timeout_cb, config->warm_up_time, 0.); warmup_watcher.data = this; + current_phase = Phase::INITIAL_IDLE; + } else { + current_phase = Phase::MAIN_DURATION; } } From 4b44362b9fb2949f85b15d3ccd8b05306e566532 Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Tue, 1 Aug 2017 20:22:20 -0400 Subject: [PATCH 07/14] minor style changes --- src/h2load.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 2f9a7fa0..16eacb81 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -1308,10 +1308,9 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, if (config->is_timing_based_mode()) { duration_watcher.data = this; - ev_timer_init(&warmup_watcher, warmup_timeout_cb, - config->warm_up_time, 0.); + ev_timer_init(&warmup_watcher, warmup_timeout_cb, config->warm_up_time, 0.); warmup_watcher.data = this; - current_phase = Phase::INITIAL_IDLE; + current_phase = Phase::INITIAL_IDLE; } else { current_phase = Phase::MAIN_DURATION; } From 46f670f8a225588e3bdde6b7d4cc34a6ada38cd9 Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Thu, 3 Aug 2017 16:15:14 -0400 Subject: [PATCH 08/14] concurrent connections are created in timing-based mode. Some safety asserts added. --- src/h2load.cc | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 16eacb81..c1162658 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -239,18 +239,25 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { make_unique(worker->next_client_id++, worker, req_todo); ++worker->nconns_made; - worker->clients.push_back(client.get()); if (client->connect() != 0) { std::cerr << "client could not connect to host" << std::endl; client->fail(); } else { + if (worker->config->is_timing_based_mode()) { + worker->clients.push_back(client.get()); + } client.release(); } worker->report_rate_progress(); } - if (worker->nconns_made >= worker->nclients) { - ev_timer_stop(worker->loop, w); + if (!worker->config->is_timing_based_mode()) { + if (worker->nconns_made >= worker->nclients) { + ev_timer_stop(worker->loop, w); + } + } else { + // To check whether all created clients are pushed correctly + assert(worker->nclients == worker->clients.size()); } } } // namespace @@ -845,6 +852,8 @@ void Client::on_stream_close(int32_t stream_id, bool success, bool final) { ++worker->stats.req_failed; ++worker->stats.req_error; } + // To avoid overflow error + assert(worker->stats.req_done <= worker->max_samples); ++worker->stats.req_done; ++req_done; } @@ -974,7 +983,9 @@ int Client::connection_made() { record_connect_time(); if (!config.timing_script) { - auto nreq = std::min(req_left, session->max_concurrent_streams()); + auto nreq = config.is_timing_based_mode() ? + std::max(req_left, session->max_concurrent_streams()) : + std::min(req_left, session->max_concurrent_streams()); for (; nreq > 0; --nreq) { if (submit_request() != 0) { process_request_failure(); @@ -1299,8 +1310,13 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, config->rate_period); timeout_watcher.data = this; - stats.req_stats.reserve(std::min(req_todo, max_samples)); - stats.client_stats.reserve(std::min(nclients, max_samples)); + if (config->is_timing_based_mode()) { + stats.req_stats.reserve(std::max(req_todo, max_samples)); + stats.client_stats.reserve(std::max(nclients, max_samples)); + } else { + stats.req_stats.reserve(std::min(req_todo, max_samples)); + stats.client_stats.reserve(std::min(nclients, max_samples)); + } sampling_init(request_times_smp, req_todo, max_samples); sampling_init(client_smp, nclients, max_samples); From b72ca0289c9db835cb6304a3f02d324e8b6f4a6a Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Fri, 4 Aug 2017 14:20:00 -0400 Subject: [PATCH 09/14] formatting issue --- src/h2load.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index c1162658..fb1a25aa 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -260,7 +260,7 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { assert(worker->nclients == worker->clients.size()); } } -} // namespace +} // namespace namespace { // Called when the duration for infinite number of requests are over @@ -983,9 +983,9 @@ int Client::connection_made() { record_connect_time(); if (!config.timing_script) { - auto nreq = config.is_timing_based_mode() ? - std::max(req_left, session->max_concurrent_streams()) : - std::min(req_left, session->max_concurrent_streams()); + auto nreq = config.is_timing_based_mode() + ? std::max(req_left, session->max_concurrent_streams()) + : std::min(req_left, session->max_concurrent_streams()); for (; nreq > 0; --nreq) { if (submit_request() != 0) { process_request_failure(); From c78159469ac0427673bdf5a374940384fd0ba984 Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Mon, 7 Aug 2017 18:58:12 -0400 Subject: [PATCH 10/14] Added a function to free a client from Worker's list of client, if the client is destroyed --- src/h2load.cc | 50 +++++++++++++++++++++++++++++++++++--------------- src/h2load.h | 2 ++ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index fb1a25aa..a483f231 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -193,6 +193,7 @@ void writecb(struct ev_loop *loop, ev_io *w, int revents) { rv = client->connect(); if (rv != 0) { client->fail(); + client->worker->free_client(client); delete client; return; } @@ -200,6 +201,7 @@ void writecb(struct ev_loop *loop, ev_io *w, int revents) { } if (rv != 0) { client->fail(); + client->worker->free_client(client); delete client; } } @@ -213,6 +215,7 @@ void readcb(struct ev_loop *loop, ev_io *w, int revents) { if (client->try_again_or_fail() == 0) { return; } + client->worker->free_client(client); delete client; return; } @@ -245,9 +248,10 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { client->fail(); } else { if (worker->config->is_timing_based_mode()) { - worker->clients.push_back(client.get()); + worker->clients.push_back(client.release()); + } else { + client.release(); } - client.release(); } worker->report_rate_progress(); } @@ -268,10 +272,12 @@ void duration_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { auto worker = static_cast(w->data); for (auto client: worker->clients) { - client->req_todo = client->req_done; // there was no finite "req_todo" - worker->stats.req_todo += client->req_todo; - client->req_inflight = 0; - client->req_left = 0; + if (client) { + client->req_todo = client->req_done; // there was no finite "req_todo" + worker->stats.req_todo += client->req_todo; + client->req_inflight = 0; + client->req_left = 0; + } } worker->current_phase = Phase::DURATION_OVER; @@ -296,15 +302,17 @@ void warmup_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { assert(worker->stats.req_done == 0); for (auto client : worker->clients) { - assert(client->req_todo == 0); - assert(client->req_left == 1); - assert(client->req_inflight == 0); - assert(client->req_started == 0); - assert(client->req_done == 0); + if (client) { + assert(client->req_todo == 0); + assert(client->req_left == 1); + assert(client->req_inflight == 0); + assert(client->req_started == 0); + assert(client->req_done == 0); - client->record_client_start_time(); - client->clear_connect_times(); - client->record_connect_start_time(); + client->record_client_start_time(); + client->clear_connect_times(); + client->record_connect_start_time(); + } } worker->current_phase = Phase::MAIN_DURATION; @@ -1339,12 +1347,24 @@ Worker::~Worker() { void Worker::stop_all_clients() { for (auto client : clients) { - if (client->session) { + if (client && client->session) { client->terminate_session(); } } } +void Worker::free_client(Client* deleted_client) { + for (auto& client : clients) { + if (client == deleted_client) { + client->req_todo = client->req_done; + stats.req_todo += client->req_todo; + auto index = &client - &clients[0]; + clients[index] = NULL; + return; + } + } +} + void Worker::run() { if (!config->is_rate_mode() && !config->is_timing_based_mode()) { for (size_t i = 0; i < nclients; ++i) { diff --git a/src/h2load.h b/src/h2load.h index 077f5752..5655583b 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -286,6 +286,8 @@ struct Worker { void report_rate_progress(); // This function calls the destructors of all the clients. void stop_all_clients(); + // This function frees a client from the list of clients for this Worker. + void free_client(Client*); }; struct Stream { From 1baf7d34b3a14a518767128afbd588cab131f8e9 Mon Sep 17 00:00:00 2001 From: Soham Sinha Date: Tue, 8 Aug 2017 17:26:37 -0400 Subject: [PATCH 11/14] Duration watcher and warmup watcher is initialised in Worker constructor. Statistic calculation is removed from duration watcher call_back, it's done in free_client. --- src/h2load.cc | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index a483f231..95ef5174 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -271,15 +271,6 @@ namespace { void duration_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { auto worker = static_cast(w->data); - for (auto client: worker->clients) { - if (client) { - client->req_todo = client->req_done; // there was no finite "req_todo" - worker->stats.req_todo += client->req_todo; - client->req_inflight = 0; - client->req_left = 0; - } - } - worker->current_phase = Phase::DURATION_OVER; std::cout << "Main benchmark duration is over for thread #" << worker->id @@ -316,9 +307,6 @@ void warmup_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { } worker->current_phase = Phase::MAIN_DURATION; - - ev_timer_init(&worker->duration_watcher, duration_timeout_cb, - worker->config->duration, 0.); ev_timer_start(worker->loop, &worker->duration_watcher); } @@ -1329,11 +1317,13 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, sampling_init(request_times_smp, req_todo, max_samples); sampling_init(client_smp, nclients, max_samples); - if (config->is_timing_based_mode()) { - duration_watcher.data = this; + ev_timer_init(&duration_watcher, duration_timeout_cb, config->duration, 0.); + duration_watcher.data = this; - ev_timer_init(&warmup_watcher, warmup_timeout_cb, config->warm_up_time, 0.); - warmup_watcher.data = this; + ev_timer_init(&warmup_watcher, warmup_timeout_cb, config->warm_up_time, 0.); + warmup_watcher.data = this; + + if (config->is_timing_based_mode()) { current_phase = Phase::INITIAL_IDLE; } else { current_phase = Phase::MAIN_DURATION; @@ -1342,6 +1332,8 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, Worker::~Worker() { ev_timer_stop(loop, &timeout_watcher); + ev_timer_stop(loop, &duration_watcher); + ev_timer_stop(loop, &warmup_watcher); ev_loop_destroy(loop); } @@ -1353,8 +1345,8 @@ void Worker::stop_all_clients() { } } -void Worker::free_client(Client* deleted_client) { - for (auto& client : clients) { +void Worker::free_client(Client *deleted_client) { + for (auto &client : clients) { if (client == deleted_client) { client->req_todo = client->req_done; stats.req_todo += client->req_todo; From c9b1c9194459d01bc2e7ee53a24bfec169d58983 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 23 Aug 2017 19:18:27 +0900 Subject: [PATCH 12/14] Fix compile error --- src/h2load.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index f57c3d32..b7af2055 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -813,10 +813,7 @@ void Client::on_stream_close(int32_t stream_id, bool success, bool final) { ++worker->stats.req_failed; } - if (sampling_should_pick(worker->request_times_smp)) { - sampling_advance_point(worker->request_times_smp); - worker->sample_req_stat(req_stat); - } + worker->sample_req_stat(req_stat); // Count up in successful cases only ++worker->request_times_smp.n; From afcd8d9ab174996d361bda5da5bade7b9e66e2c6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 23 Aug 2017 19:19:00 +0900 Subject: [PATCH 13/14] clang-format --- src/h2load.cc | 56 +++++++++++++++++++++++++-------------------------- src/h2load.h | 18 +++++++++-------- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index b7af2055..30f29a89 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -243,7 +243,7 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { assert(worker->nclients == worker->clients.size()); } } -} // namespace +} // namespace namespace { // Called when the duration for infinite number of requests are over @@ -257,7 +257,7 @@ void duration_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { worker->stop_all_clients(); std::cout << "Stopped all clients for thread #" << worker->id << std::endl; } -} // namespace +} // namespace namespace { // Called when the warmup duration for infinite number of requests are over @@ -286,7 +286,7 @@ void warmup_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { } worker->current_phase = Phase::MAIN_DURATION; - + ev_timer_start(worker->loop, &worker->duration_watcher); } } // namespace @@ -449,15 +449,15 @@ int Client::make_socket(addrinfo *addr) { int Client::connect() { int rv; - if (!worker->config->is_timing_based_mode() - || worker->current_phase == Phase::MAIN_DURATION) { + if (!worker->config->is_timing_based_mode() || + worker->current_phase == Phase::MAIN_DURATION) { record_client_start_time(); clear_connect_times(); record_connect_start_time(); } else if (worker->current_phase == Phase::INITIAL_IDLE) { worker->current_phase = Phase::WARM_UP; - std::cout << "Warm-up started for thread #" << worker->id - << "." << std::endl; + std::cout << "Warm-up started for thread #" << worker->id << "." + << std::endl; ev_timer_start(worker->loop, &worker->warmup_watcher); } @@ -646,7 +646,8 @@ void Client::process_request_failure() { if (req_inflight == 0) { terminate_session(); } - std::cout << "Process Request Failure:" << worker->stats.req_failed << std::endl; + std::cout << "Process Request Failure:" << worker->stats.req_failed + << std::endl; } namespace { @@ -749,7 +750,7 @@ void Client::on_header(int32_t stream_id, const uint8_t *name, size_t namelen, break; } } - + if (status >= 200 && status < 300) { ++worker->stats.status[2]; stream.status_success = 1; @@ -1227,7 +1228,7 @@ void Client::record_client_start_time() { if (recorded(cstat.client_start_time)) { return; } - + cstat.client_start_time = std::chrono::steady_clock::now(); } @@ -1347,7 +1348,7 @@ void Worker::run() { client.release(); } } - } else if (config->is_rate_mode()){ + } else if (config->is_rate_mode()) { ev_timer_again(loop, &timeout_watcher); // call callback so that we don't waste the first rate_period @@ -1384,8 +1385,8 @@ void Worker::sample_client_stat(ClientStat *cstat) { } void Worker::report_progress() { - if (id != 0 || config->is_rate_mode() || stats.req_done % progress_interval - || config->is_timing_based_mode()) { + if (id != 0 || config->is_rate_mode() || stats.req_done % progress_interval || + config->is_timing_based_mode()) { return; } @@ -1730,7 +1731,7 @@ std::unique_ptr create_worker(uint32_t id, SSL_CTX *ssl_ctx, if (config.is_timing_based_mode()) { std::cout << "spawning thread #" << id << ": " << nclients << " total client(s). Timing-based test with " - << config.warm_up_time << "s of warm-up time and " + << config.warm_up_time << "s of warm-up time and " << config.duration << "s of main duration for measurements." << std::endl; } else { @@ -1741,12 +1742,12 @@ std::unique_ptr create_worker(uint32_t id, SSL_CTX *ssl_ctx, if (config.is_rate_mode()) { return make_unique(id, ssl_ctx, nreqs, nclients, rate, max_samples, - &config); + &config); } else { // Here rate is same as client because the rate_timeout callback // will be called only once - return make_unique(id, ssl_ctx, nreqs, nclients, nclients, max_samples, - &config); + return make_unique(id, ssl_ctx, nreqs, nclients, nclients, + max_samples, &config); } } } // namespace @@ -2029,9 +2030,9 @@ int main(int argc, char **argv) { {"warm-up-time", required_argument, &flag, 9}, {nullptr, 0, nullptr, 0}}; int option_index = 0; - auto c = - getopt_long(argc, argv, "hvW:c:d:m:n:p:t:w:H:i:r:T:N:D:B:", long_options, - &option_index); + auto c = getopt_long(argc, argv, + "hvW:c:d:m:n:p:t:w:H:i:r:T:N:D:B:", long_options, + &option_index); if (c == -1) { break; } @@ -2345,8 +2346,7 @@ int main(int argc, char **argv) { if (config.nreqs == 0 && !config.is_timing_based_mode()) { std::cerr << "-n: the number of requests must be strictly greater than 0," - << "timing-based test is not being run." - << std::endl; + << "timing-based test is not being run." << std::endl; exit(EXIT_FAILURE); } @@ -2369,7 +2369,8 @@ int main(int argc, char **argv) { // With timing script, we don't distribute config.nreqs to each // client or thread. - if (!config.timing_script && config.nreqs < config.nclients && !config.is_timing_based_mode()) { + if (!config.timing_script && config.nreqs < config.nclients && + !config.is_timing_based_mode()) { std::cerr << "-n, -c: the number of requests must be greater than or " << "equal to the clients." << std::endl; exit(EXIT_FAILURE); @@ -2377,16 +2378,14 @@ int main(int argc, char **argv) { if (config.nclients < config.nthreads) { std::cerr << "-c, -t: the number of clients must be greater than or equal " - << "to the number of threads." - << std::endl; + << "to the number of threads." << std::endl; exit(EXIT_FAILURE); } if (config.is_timing_based_mode()) { if (config.nreqs != 0) { std::cerr << "-n: the number of requests needs to be zero (0) for timing-" - << "based test. Default value is 1." - << std::endl; + << "based test. Default value is 1." << std::endl; exit(EXIT_FAILURE); } } @@ -2741,7 +2740,8 @@ int main(int argc, char **argv) { bps = stats.bytes_total / config.duration; } else { auto secd = std::chrono::duration_cast< - std::chrono::duration>(duration); + std::chrono::duration>( + duration); rps = stats.req_success / secd.count(); bps = stats.bytes_total / secd.count(); } diff --git a/src/h2load.h b/src/h2load.h index 55b1130d..69d9a6ee 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -221,12 +221,12 @@ struct Stats { enum ClientState { CLIENT_IDLE, CLIENT_CONNECTED }; // This type tells whether the client is in warmup phase or not or is over -enum class Phase { - INITIAL_IDLE, // Initial idle state before warm-up phase - WARM_UP, // Warm up phase when no measurements are done +enum class Phase { + INITIAL_IDLE, // Initial idle state before warm-up phase + WARM_UP, // Warm up phase when no measurements are done MAIN_DURATION, // Main measurement phase; if timing-based // test is not run, this is the default phase - DURATION_OVER // This phase occurs after the measurements are over + DURATION_OVER // This phase occurs after the measurements are over }; struct Client; @@ -264,11 +264,13 @@ struct Worker { ev_timer timeout_watcher; // The next client ID this worker assigns uint32_t next_client_id; - // Keeps track of the current phase (for timing-based experiment) for the worker + // Keeps track of the current phase (for timing-based experiment) for the + // worker Phase current_phase; // We need to keep track of the clients in order to stop them when needed - std::vector clients; - // This is only active when there is not a bounded number of requests specified + std::vector clients; + // This is only active when there is not a bounded number of requests + // specified ev_timer duration_watcher; ev_timer warmup_watcher; @@ -284,7 +286,7 @@ struct Worker { // This function calls the destructors of all the clients. void stop_all_clients(); // This function frees a client from the list of clients for this Worker. - void free_client(Client*); + void free_client(Client *); }; struct Stream { From bcda1c240925718fb5e9c37459b14e8fe6783d51 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 23 Aug 2017 19:22:23 +0900 Subject: [PATCH 14/14] Fix assertion failure --- src/h2load.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 30f29a89..258f3ede 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -822,8 +822,6 @@ void Client::on_stream_close(int32_t stream_id, bool success, bool final) { ++worker->stats.req_failed; ++worker->stats.req_error; } - // To avoid overflow error - assert(worker->stats.req_done <= worker->max_samples); ++worker->stats.req_done; ++req_done; }