From: Tomek Mrugalski Date: Fri, 24 Aug 2012 12:39:56 +0000 (+0200) Subject: [2040] Changes after review X-Git-Tag: trac2351_base~117^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8c7391beed6e06f9bbb9957defc8018e6d6953fc;p=thirdparty%2Fkea.git [2040] Changes after review --- diff --git a/tests/tools/dhcp-ubench/benchmark.cc b/tests/tools/dhcp-ubench/benchmark.cc index 8e67c7f961..c987bf5013 100644 --- a/tests/tools/dhcp-ubench/benchmark.cc +++ b/tests/tools/dhcp-ubench/benchmark.cc @@ -36,12 +36,11 @@ uBenchmark::uBenchmark(uint32_t iterations, const std::string& dbname, const std::string& pass /* = "" */) :num_(iterations), sync_(sync), verbose_(verbose), hostname_(host), user_(user), passwd_(pass), dbname_(dbname), - compiled_stmt_(true) + hitratio_(0.9f), compiled_stmt_(true) { /// @todo: make compiled statements a configurable parameter - /// @todo: convert this to user-configurable parameter - hitratio_ = 0.9f; + /// @todo: convert hitratio_ to user-configurable parameter memset(ts_, 0, sizeof(ts_)); } @@ -57,9 +56,10 @@ void uBenchmark::usage() { cout << " -u username - specifies MySQL user name (MySQL backend only)" << endl; cout << " -p password - specifies MySQL passwod (MySQL backend only)" << endl; cout << " -f name - database or filename (MySQL, SQLite and memfile)" << endl; - cout << " -n integer - number of test repetitions (MySQL, SQLite and memfile)" << endl; + cout << " -n integer - number of test iterations (MySQL, SQLite and memfile)" << endl; cout << " -s yes|no - synchronous/asynchronous operation (MySQL, SQLite and memfile)" << endl; cout << " -v yes|no - verbose mode (MySQL, SQLite and memfile)" << endl; + cout << " -c yes|no - compiled statements (MySQL and SQLite)" << endl; exit(EXIT_FAILURE); } @@ -85,9 +85,10 @@ void uBenchmark::parseCmdline(int argc, char* const argv[]) { break; case 'n': try { - num_ = boost::lexical_cast(optarg); + num_ = boost::lexical_cast(optarg); } catch (const boost::bad_lexical_cast &) { - cerr << "Failed to iterations (-n option)." << endl; + cerr << "Failed to parse number of iterations (-n option):" + << optarg << endl; usage(); } break; @@ -111,9 +112,9 @@ void uBenchmark::failure(const char* operation) { throw string(operation); } -void uBenchmark::print_clock(const std::string& operation, uint32_t num, - const struct timespec& before, - const struct timespec& after) { +void uBenchmark::printClock(const std::string& operation, uint32_t num, + const struct timespec& before, + const struct timespec& after) { long int tv_sec = after.tv_sec - before.tv_sec; long int tv_nsec = after.tv_nsec - before.tv_nsec; @@ -149,19 +150,19 @@ int uBenchmark::run() { try { connect(); - ts_[0] = get_time(); + ts_[0] = getTime(); createLease4Test(); - ts_[1] = get_time(); + ts_[1] = getTime(); searchLease4Test(); - ts_[2] = get_time(); + ts_[2] = getTime(); updateLease4Test(); - ts_[3] = get_time(); + ts_[3] = getTime(); deleteLease4Test(); - ts_[4] = get_time(); + ts_[4] = getTime(); disconnect(); @@ -170,15 +171,15 @@ int uBenchmark::run() { return (-1); } - print_clock("Create leases4", num_, ts_[0], ts_[1]); - print_clock("Search leases4", num_, ts_[1], ts_[2]); - print_clock("Update leases4", num_, ts_[2], ts_[3]); - print_clock("Delete leases4", num_, ts_[3], ts_[4]); + printClock("Create leases4", num_, ts_[0], ts_[1]); + printClock("Search leases4", num_, ts_[1], ts_[2]); + printClock("Update leases4", num_, ts_[2], ts_[3]); + printClock("Delete leases4", num_, ts_[3], ts_[4]); return (0); } -struct timespec uBenchmark::get_time() { +struct timespec uBenchmark::getTime() { struct timespec ts; #ifdef __MACH__ // OS X does not have clock_gettime, use clock_get_time diff --git a/tests/tools/dhcp-ubench/benchmark.h b/tests/tools/dhcp-ubench/benchmark.h index 6fee87009d..6001e81297 100644 --- a/tests/tools/dhcp-ubench/benchmark.h +++ b/tests/tools/dhcp-ubench/benchmark.h @@ -67,11 +67,11 @@ public: /// /// That benchmark method will be called first. /// It is expected to create specific number of leases, - /// as specified by \ref Num_ parameter. Following + /// as specified by \ref num_ parameter. Following /// methods (searchLease4Test(), updateLease4Test(), /// and deleteLease4Test()) assume that lease creation /// is successful. The benchmark is expected to create leases - /// starting from BASE_ADDR4 and ending on BASE_ADDR4 + Num_. + /// starting from BASE_ADDR4 and ending on BASE_ADDR4 + num_. /// /// The implementation is provided by the DB-specific class. virtual void createLease4Test() = 0; @@ -80,10 +80,10 @@ public: /// /// This is the second benchmark in a series of four. /// It is called after createLease4Test(), so it expects that the - /// database is populated with at least \ref Num_ leases. - /// It repeats search for a lease Num_ times. + /// database is populated with at least \ref num_ leases. + /// It repeats search for a lease num_ times. /// - /// The algorithm randomly picks a lease with HitRatio_ (typically 90%) + /// The algorithm randomly picks a lease with \ref hitratio_ (typically 90%) /// chance of finding a lease. During typical DHCP operation the server /// sometimes wants to check if specific lease is assigned or not and the /// lease is sometimes not present (e.g. when randomly trying to pick a new @@ -98,15 +98,15 @@ public: /// /// This is the third benchmark in a series of four. /// It is called after createLease4Test(), so it expects that the - /// database is populated with at least \ref Num_ leases. + /// database is populated with at least \ref num_ leases. /// /// In a normal DHCP operation, search and update operations are used /// together, but for the benchmarking purposes they are executed /// separately here. Once a lease is found, it is being updated. Typically /// the update is just changing lease expiration timers, so that is what - /// the test does. It exploits the fact that there are Num_ leases + /// the test does. It exploits the fact that there are num_ leases /// in the database, so it picks randomly an address from - /// BASE_ADDR4 ... BASE_ADDR4+Num_ range and has a guarantee for the lease + /// BASE_ADDR4 ... BASE_ADDR4 + num_ range and has a guarantee for the lease /// to be present. /// /// The implementation is provided by the DB-specific class. @@ -116,9 +116,9 @@ public: /// /// This is the last benchmark in a series of four. /// It is called after createLease4Test(), so it expects that the - /// database is populated with at least \ref Num_ leases. + /// database is populated with at least \ref num_ leases. /// - /// It is expected to iteratively delete all Num_ leases from + /// It is expected to iteratively delete all num_ leases from /// the database. /// /// The implementation is provided by the DB-specific class. @@ -143,9 +143,9 @@ public: /// @param num number or iterations (used for statistics) /// @param before timestamp before execution /// @param after timestamp after execution - void print_clock(const std::string& operation, uint32_t num, - const struct timespec& before, - const struct timespec& after); + void printClock(const std::string& operation, uint32_t num, + const struct timespec& before, + const struct timespec& after); /// @brief Main benchmark execution routine /// @@ -173,7 +173,7 @@ protected: void usage(); /// @brief a wrapper around OS-specific method for getting time - struct timespec get_time(); + struct timespec getTime(); /// Number of operations (e.g. insert lease num times) uint32_t num_; diff --git a/tests/tools/dhcp-ubench/memfile_ubench.cc b/tests/tools/dhcp-ubench/memfile_ubench.cc index cd589b4a37..6e92585d72 100644 --- a/tests/tools/dhcp-ubench/memfile_ubench.cc +++ b/tests/tools/dhcp-ubench/memfile_ubench.cc @@ -86,43 +86,43 @@ protected: void writeLease(Lease4Ptr lease); /// Name of the lease file. - std::string Filename_; + std::string filename_; /// should we do flush after each operation? - bool Sync_; + bool sync_; /// File handle to the open lease file. - FILE * File_; + FILE * file_; /// Hash table for IPv4 leases IPv4Hash ip4Hash_; }; memfile_LeaseMgr::memfile_LeaseMgr(const std::string& filename, bool sync) - : Filename_(filename), Sync_(sync) { - File_ = fopen(filename.c_str(), "w"); - if (!File_) { + : filename_(filename), sync_(sync) { + file_ = fopen(filename.c_str(), "w"); + if (!file_) { throw "Failed to create file " + filename; } } memfile_LeaseMgr::~memfile_LeaseMgr() { - fclose(File_); + fclose(file_); } void memfile_LeaseMgr::writeLease(Lease4Ptr lease) { - fprintf(File_, "lease %d {\n hw-addr ", lease->addr); + fprintf(file_, "lease %d {\n hw-addr ", lease->addr); for (std::vector::const_iterator it = lease->hwaddr.begin(); it != lease->hwaddr.end(); ++it) { - fprintf(File_,"%02x:", *it); + fprintf(file_, "%02x:", *it); } - fprintf(File_, ";\n client-id "); + fprintf(file_, ";\n client-id "); for (std::vector::const_iterator it = lease->client_id.begin(); it != lease->client_id.end(); ++it) { - fprintf(File_, "%02x:", *it); + fprintf(file_, "%02x:", *it); } - fprintf(File_, ";\n valid-lifetime %d;\n recycle-time %d;\n" + fprintf(file_, ";\n valid-lifetime %d;\n recycle-time %d;\n" " cltt %d;\n pool-id %d;\n fixed %s; hostname %s;\n" " fqdn_fwd %s;\n fqdn_rev %s;\n};\n", lease->valid_lft, lease->recycle_time, (int)lease->cltt, @@ -130,9 +130,9 @@ void memfile_LeaseMgr::writeLease(Lease4Ptr lease) { lease->hostname.c_str(), lease->fqdn_fwd?"true":"false", lease->fqdn_rev?"true":"false"); - if (Sync_) { - fflush(File_); - fsync(fileno(File_)); + if (sync_) { + fflush(file_); + fsync(fileno(file_)); } } @@ -219,21 +219,22 @@ void memfile_uBenchmark::createLease4Test() { bool fqdn_fwd = true; // Let's pretend to do AAAA update bool fqdn_rev = true; // Let's pretend to do PTR update - printf("CREATE: "); + cout << "CREATE: "; // While we could put the data directly into vector, I would like to // keep the code as similar to other benchmarks as possible - for (uint8_t i = 0; i < hwaddr_len; i++) { - hwaddr_tmp[i] = 65 + i; + for (uint8_t i = 0; i < hwaddr_len; ++i) { + hwaddr_tmp[i] = 'A' + i; // let's make hwaddr consisting of letter } - vector hwaddr(hwaddr_tmp, hwaddr_tmp + 19); + vector hwaddr(hwaddr_tmp, hwaddr_tmp + hwaddr_len - 1); for (uint8_t i = 0; i < client_id_len; i++) { - client_id_tmp[i] = 33 + i; + client_id_tmp[i] = 33 + i; // 33 is being the first, non whitespace + // printable ASCII character } - vector client_id(client_id_tmp, client_id_tmp + 19); + vector client_id(client_id_tmp, client_id_tmp + client_id_len - 1); - for (uint32_t i = 0; i < num_; i++) { + for (uint32_t i = 0; i < num_; ++i) { cltt++; @@ -254,13 +255,13 @@ void memfile_uBenchmark::createLease4Test() { failure("addLease() failed"); } else { if (verbose_) { - printf("."); + cout << "."; } }; addr++; } - printf("\n"); + cout << endl; } void memfile_uBenchmark::searchLease4Test() { @@ -268,25 +269,18 @@ void memfile_uBenchmark::searchLease4Test() { throw "No LeaseMgr instantiated."; } - // This formula should roughly find something a lease in 90% cases - float hitRatio = 0.5; - - printf("RETRIEVE: "); + cout << "RETRIEVE: "; for (uint32_t i = 0; i < num_; i++) { - uint32_t x = BASE_ADDR4 + random() % int(num_ / hitRatio); + uint32_t x = BASE_ADDR4 + random() % int(num_ / hitratio_); Lease4Ptr lease = leaseMgr_->getLease(x); if (verbose_) { - if (lease) { - printf("."); - } else { - printf("X"); - } + cout << (lease?".":"X"); } } - printf("\n"); + cout << endl; } void memfile_uBenchmark::updateLease4Test() { @@ -294,7 +288,7 @@ void memfile_uBenchmark::updateLease4Test() { throw "No LeaseMgr instantiated."; } - printf("UPDATE: "); + cout << "UPDATE: "; time_t cltt = time(NULL); @@ -309,11 +303,11 @@ void memfile_uBenchmark::updateLease4Test() { failure(tmp.str().c_str()); } if (verbose_) { - printf("."); + cout << "."; } } - printf("\n"); + cout << endl; } void memfile_uBenchmark::deleteLease4Test() { @@ -321,7 +315,7 @@ void memfile_uBenchmark::deleteLease4Test() { throw "No LeaseMgr instantiated."; } - printf("DELETE: "); + cout << "DELETE: "; for (uint32_t i = 0; i < num_; i++) { @@ -333,11 +327,11 @@ void memfile_uBenchmark::deleteLease4Test() { failure(tmp.str().c_str()); } if (verbose_) { - printf("."); + cout << "."; } } - printf("\n"); + cout << endl; } void memfile_uBenchmark::printInfo() { diff --git a/tests/tools/dhcp-ubench/mysql_ubench.cc b/tests/tools/dhcp-ubench/mysql_ubench.cc index ca1984c47f..a3076597c4 100644 --- a/tests/tools/dhcp-ubench/mysql_ubench.cc +++ b/tests/tools/dhcp-ubench/mysql_ubench.cc @@ -101,7 +101,7 @@ void MySQL_uBenchmark::createLease4Test() { sprintf(cltt, "2012-07-11 15:43:00"); cltt_len = strlen(cltt); - uint32_t pool_id = 1000; // Let's use pools 0-99 + uint32_t pool_id = 1000; // Let's use pool-ids greater than zero bool fixed = false; char hostname[] = "foo"; // Will generate it dynamically @@ -111,17 +111,18 @@ void MySQL_uBenchmark::createLease4Test() { bool fqdn_fwd = true; // Let's pretend to do AAAA update bool fqdn_rev = true; // Let's pretend to do PTR update - printf("CREATE: "); + cout << "CREATE: "; - for (uint8_t i = 0; i < 20; i++) { - hwaddr[i] = 65 + i; + for (uint8_t i = 0; i < hwaddr_len; i++) { + hwaddr[i] = 'A' + i; // let's make hwaddr consisting of letters } - hwaddr[19] = 0; + hwaddr[19] = 0; // make it is null-terminated - for (uint8_t i = 0; i < 128; i++) { - client_id[i] = 33 + i; + for (uint8_t i = 0; i < client_id_len; i++) { + client_id[i] = 33 + i; // 33 is being the first, non whitespace + // printable ASCII character } - client_id[127] = 0; + client_id[127] = 0; // make it is null-terminated MYSQL_STMT * stmt = NULL; MYSQL_BIND bind[11]; // 11 parameters in the insert statement @@ -262,7 +263,7 @@ void MySQL_uBenchmark::createLease4Test() { } if (verbose_) { - printf("."); + cout << "."; } } @@ -272,7 +273,7 @@ void MySQL_uBenchmark::createLease4Test() { } } - printf("\n"); + cout << endl; } void MySQL_uBenchmark::searchLease4Test() { @@ -280,7 +281,7 @@ void MySQL_uBenchmark::searchLease4Test() { throw "Not connected to MySQL server."; } - printf("RETRIEVE: "); + cout << "RETRIEVE: "; uint32_t addr = 0; @@ -348,12 +349,12 @@ void MySQL_uBenchmark::searchLease4Test() { mysql_free_result(result); if (verbose_) { - printf("."); // hit + cout << "."; // hit } } else { if (verbose_) { - printf("x"); // miss + cout << "x"; // miss } } } else { @@ -438,7 +439,7 @@ void MySQL_uBenchmark::searchLease4Test() { if (mysql_stmt_bind_result(stmt, response)) { - printf("Error: %s\n", mysql_stmt_error(stmt)); + cout << "Error:" << mysql_stmt_error(stmt) << endl; failure("mysql_stmt_bind_result() failed"); } int num_rows = 0; @@ -456,11 +457,11 @@ void MySQL_uBenchmark::searchLease4Test() { if (num_rows) { if (verbose_) { - printf("."); // hit + cout << "."; // hit } } else { if (verbose_) { - printf("x"); // miss + cout << "X"; // miss } } @@ -473,7 +474,7 @@ void MySQL_uBenchmark::searchLease4Test() { } } - printf("\n"); + cout << endl; } void MySQL_uBenchmark::updateLease4Test() { @@ -481,7 +482,7 @@ void MySQL_uBenchmark::updateLease4Test() { throw "Not connected to MySQL server."; } - printf("UPDATE: "); + cout << "UPDATE: "; uint32_t valid_lft = 1002; // just some dummy value char cltt[] = "now()"; @@ -541,7 +542,7 @@ void MySQL_uBenchmark::updateLease4Test() { } if (verbose_) { - printf("."); + cout << "."; } } @@ -551,7 +552,7 @@ void MySQL_uBenchmark::updateLease4Test() { } } - printf("\n"); + cout << endl; } void MySQL_uBenchmark::deleteLease4Test() { @@ -559,7 +560,7 @@ void MySQL_uBenchmark::deleteLease4Test() { throw "Not connected to MySQL server."; } - printf("DELETE: "); + cout << "DELETE: "; uint32_t addr = 0; @@ -612,7 +613,7 @@ void MySQL_uBenchmark::deleteLease4Test() { } if (verbose_) { - printf("."); + cout << "."; } } @@ -622,7 +623,7 @@ void MySQL_uBenchmark::deleteLease4Test() { } } - printf("\n"); + cout << endl; } void MySQL_uBenchmark::printInfo() { diff --git a/tests/tools/dhcp-ubench/mysql_ubench.h b/tests/tools/dhcp-ubench/mysql_ubench.h index 822b4eed56..c9fcc7c39a 100644 --- a/tests/tools/dhcp-ubench/mysql_ubench.h +++ b/tests/tools/dhcp-ubench/mysql_ubench.h @@ -33,7 +33,7 @@ public: /// /// Synchronous operation means using InnDB, async is MyISAM. /// - /// @param hostname Name of the hostname to connect to + /// @param hostname Name of the host to connect to /// @param user usename used during MySQL connection /// @param pass password used during MySQL connection /// @param db name of the database to connect to diff --git a/tests/tools/dhcp-ubench/sqlite_ubench.cc b/tests/tools/dhcp-ubench/sqlite_ubench.cc index 7832d3aac6..1018b2b337 100644 --- a/tests/tools/dhcp-ubench/sqlite_ubench.cc +++ b/tests/tools/dhcp-ubench/sqlite_ubench.cc @@ -58,6 +58,8 @@ void SQLite_uBenchmark::disconnect() { if (db_) { sqlite3_close(db_); db_ = NULL; + } else { + throw "Can't close SQLite connection: it was never open."; } } @@ -81,7 +83,7 @@ void SQLite_uBenchmark::createLease4Test() { bool fqdn_fwd = true; // Let's pretend to do AAAA update bool fqdn_rev = true; // Let's pretend to do PTR update - printf("CREATE: "); + cout << "CREATE: "; for (uint8_t i = 0; i < hwaddr_len; i++) { hwaddr[i] = 65 + i; @@ -126,7 +128,6 @@ void SQLite_uBenchmark::createLease4Test() { addr, hwaddr, client_id, valid_lft, recycle_time, cltt, pool_id, (fixed?"true":"false"), hostname.c_str(), (fqdn_fwd?"true":"false"), (fqdn_rev?"true":"false")); - // printf("QUERY=[%s]\n", query); int result = sqlite3_exec(db_, query, NULL, 0, &errorMsg); @@ -204,7 +205,7 @@ void SQLite_uBenchmark::createLease4Test() { } if (verbose_) { - printf("."); + cout << "."; } } @@ -215,23 +216,30 @@ void SQLite_uBenchmark::createLease4Test() { } } - printf("\n"); + cout << endl; } -static int search_callback(void *counter, int /*argc*/, char** /*argv*/, - char** /*azColName*/){ +static int search_callback(void *counter, int argc, char** argv, + char** azColName){ int* cnt = static_cast(counter); (*cnt)++; -#if 0 - int i; - for(i=0; i