From ca7b312a177207e478e9ba5878282cb35ad42e95 Mon Sep 17 00:00:00 2001 From: Ajay Seth Date: Mon, 16 Jul 2018 17:07:54 -0700 Subject: [PATCH 1/5] Update test to use a Storage file with more data for a better assessment of read-time. --- OpenSim/Common/Test/testC3DFileAdapter.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/OpenSim/Common/Test/testC3DFileAdapter.cpp b/OpenSim/Common/Test/testC3DFileAdapter.cpp index 152341d899..fcc6762cde 100644 --- a/OpenSim/Common/Test/testC3DFileAdapter.cpp +++ b/OpenSim/Common/Test/testC3DFileAdapter.cpp @@ -76,15 +76,16 @@ void test(const std::string filename) { using namespace std; std::clock_t startTime = std::clock(); - Storage("test.sto"); + Storage("std_walking2_grfs.sto"); double loadTime = 1.e3*(std::clock() - startTime) / CLOCKS_PER_SEC; // The walking C3D files included in this test should not take more - // than 20x the time to load the 'test.sto' Storage on most hardware. - // We make the max time 50x to account for other potential slowdowns - // on CI machines. + // than 10x the time to load the 'std_walking2_grfs.sto' Storage on most + // hardware. We make the max time 50x to account for other potential + // slowdowns on CI machines. const double MaximumLoadTimeInMS = 50*loadTime; + startTime = std::clock(); auto tables = C3DFileAdapter::read(filename, C3DFileAdapter::ForceLocation::OriginOfForcePlate); From a55b4c8191fdb8c1a62183daf82d5ce8014eb333 Mon Sep 17 00:00:00 2001 From: Ajay Seth Date: Tue, 17 Jul 2018 11:43:47 -0700 Subject: [PATCH 2/5] Convert to use std::chrono::steady_clock which is guaranteed to be monotonically increasing instead of std::clock() for timing. --- OpenSim/Common/Test/testC3DFileAdapter.cpp | 74 +++++++++++++--------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/OpenSim/Common/Test/testC3DFileAdapter.cpp b/OpenSim/Common/Test/testC3DFileAdapter.cpp index fcc6762cde..08ce1d9836 100644 --- a/OpenSim/Common/Test/testC3DFileAdapter.cpp +++ b/OpenSim/Common/Test/testC3DFileAdapter.cpp @@ -31,6 +31,8 @@ #include #include +static double MaximumLoadTimeInS = SimTK::NaN; + template void compare_tables(const OpenSim::TimeSeriesTable_& table1, const OpenSim::TimeSeriesTable_& table2, @@ -74,30 +76,22 @@ void downsample_table(OpenSim::TimeSeriesTable_& table, void test(const std::string filename) { using namespace OpenSim; using namespace std; + using namespace std::chrono; - std::clock_t startTime = std::clock(); - Storage("std_walking2_grfs.sto"); - double loadTime = 1.e3*(std::clock() - startTime) / CLOCKS_PER_SEC; - - // The walking C3D files included in this test should not take more - // than 10x the time to load the 'std_walking2_grfs.sto' Storage on most - // hardware. We make the max time 50x to account for other potential - // slowdowns on CI machines. - const double MaximumLoadTimeInMS = 50*loadTime; - - startTime = std::clock(); + steady_clock::time_point startTime = steady_clock::now(); auto tables = C3DFileAdapter::read(filename, C3DFileAdapter::ForceLocation::OriginOfForcePlate); - loadTime = 1.e3*(std::clock() - startTime) / CLOCKS_PER_SEC; + steady_clock::time_point t2 = steady_clock::now(); + double loadTime = duration_cast>(t2 - startTime).count(); cout << "\tC3DFileAdapter '" << filename << "' loaded in " - << loadTime << "ms" << endl; + << loadTime << "s" << endl; #ifdef NDEBUG - ASSERT(loadTime < MaximumLoadTimeInMS, __FILE__, __LINE__, + ASSERT(loadTime < MaximumLoadTimeInS, __FILE__, __LINE__, "Unable to load '" + filename + "' within " + - to_string(MaximumLoadTimeInMS) + "ms."); + to_string(MaximumLoadTimeInS) + "s."); #endif auto& marker_table = tables.at("markers"); @@ -117,10 +111,11 @@ void test(const std::string filename) { marker_table->updTableMetaData().setValueForKey("Units", std::string{"mm"}); TRCFileAdapter trc_adapter{}; - std::clock_t t0 = std::clock(); + steady_clock::time_point t0 = steady_clock::now(); trc_adapter.write(*marker_table, marker_file); + t2 = steady_clock::now(); cout << "\tWrote '" << marker_file << "' in " - << 1.e3*(std::clock() - t0) / CLOCKS_PER_SEC << "ms" << endl; + << duration_cast>(t2 - t0).count() << "s" << endl; ASSERT(force_table->getNumRows() > 0, __FILE__, __LINE__, "Failed to read forces data from " + filename); @@ -128,17 +123,19 @@ void test(const std::string filename) { force_table->updTableMetaData().setValueForKey("Units", std::string{"mm"}); STOFileAdapter sto_adapter{}; - t0 = std::clock(); + t0 = steady_clock::now(); sto_adapter.write((force_table->flatten()), forces_file); + t2 = steady_clock::now(); cout << "\tWrote'" << forces_file << "' in " - << 1.e3*(std::clock() - t0) / CLOCKS_PER_SEC << "ms" << endl; + << duration_cast>(t2 - t0).count() << "s" << endl; // Verify that marker data was written out and can be read in - t0 = std::clock(); + t0 = steady_clock::now(); auto markers = trc_adapter.read(marker_file); auto std_markers = trc_adapter.read("std_" + marker_file); + t2 = steady_clock::now(); cout << "\tRead'" << marker_file << "' and its standard in " - << 1.e3*(std::clock() - t0) / CLOCKS_PER_SEC << "ms" << endl; + << duration_cast>(t2 - t0).count() << "s" << endl; // Compare C3DFileAdapter read-in and written marker data compare_tables(markers, *marker_table); @@ -161,20 +158,19 @@ void test(const std::string filename) { cout << "\tForces " << forces_file << " equivalent to standard." << endl; - - t0 = std::clock(); + t0 = steady_clock::now(); // Reread in C3D file with forces resolved to the COP auto tables2 = C3DFileAdapter::read(filename, C3DFileAdapter::ForceLocation::CenterOfPressure); - - loadTime = 1.e3*(std::clock() - t0) / CLOCKS_PER_SEC; + t2 = steady_clock::now(); + loadTime = duration_cast>(t2 - t0).count(); cout << "\tC3DFileAdapter '" << filename << "' read with forces at COP in " - << loadTime << "ms" << endl; + << loadTime << "s" << endl; #ifdef NDEBUG - ASSERT(loadTime < MaximumLoadTimeInMS, __FILE__, __LINE__, + ASSERT(loadTime < MaximumLoadTimeInS, __FILE__, __LINE__, "Unable to load '" + filename + "' within " + - to_string(MaximumLoadTimeInMS) + "ms."); + to_string(MaximumLoadTimeInS) + "s."); #endif auto& force_table_cop = tables2.at("forces"); @@ -191,11 +187,31 @@ void test(const std::string filename) { cout << "\tcop_" << forces_file << " is equivalent to its standard."<< endl; + t2 = steady_clock::now(); cout << "\ttestC3DFileAdapter '" << filename << "' completed in " - << 1.e3*(std::clock() - startTime) / CLOCKS_PER_SEC << "ms" << endl; + << duration_cast>(t2-startTime).count() << "s" << endl; } int main() { + using namespace OpenSim; + using namespace std; + using namespace std::chrono; + + steady_clock::time_point startTime = steady_clock::now(); + Storage("test.sto"); + steady_clock::time_point t2 = steady_clock::now(); + + double loadTime = duration_cast>(t2 - startTime).count(); + + // The walking C3D files included in this test should not take more + // than 20x the time to load the 'test.sto' Storage on most + // hardware. We make the max time 40x to account for other potential + // slowdowns on CI machines. + MaximumLoadTimeInS = 20 * loadTime; + + cout << "Platform specific Maximum load time is " + << MaximumLoadTimeInS << "s" << endl; + std::vector filenames{}; filenames.push_back("walking2.c3d"); filenames.push_back("walking5.c3d"); From 0f20ac05816f6f6c16929c03c04ff74c6c75ff6c Mon Sep 17 00:00:00 2001 From: Ajay Seth Date: Tue, 17 Jul 2018 12:30:31 -0700 Subject: [PATCH 3/5] Revert back to using std_walking2_grfs.sto as the timing benchmark. --- OpenSim/Common/Test/testC3DFileAdapter.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OpenSim/Common/Test/testC3DFileAdapter.cpp b/OpenSim/Common/Test/testC3DFileAdapter.cpp index 08ce1d9836..bbbad1d3c6 100644 --- a/OpenSim/Common/Test/testC3DFileAdapter.cpp +++ b/OpenSim/Common/Test/testC3DFileAdapter.cpp @@ -198,16 +198,16 @@ int main() { using namespace std::chrono; steady_clock::time_point startTime = steady_clock::now(); - Storage("test.sto"); + Storage("std_walking2_grfs.sto"); steady_clock::time_point t2 = steady_clock::now(); double loadTime = duration_cast>(t2 - startTime).count(); // The walking C3D files included in this test should not take more - // than 20x the time to load the 'test.sto' Storage on most - // hardware. We make the max time 40x to account for other potential + // than 10x the time to load the 'std_walking2_grfs.sto' Storage on most + // hardware. We make the max time 50x to account for other potential // slowdowns on CI machines. - MaximumLoadTimeInS = 20 * loadTime; + MaximumLoadTimeInS = 50 * loadTime; cout << "Platform specific Maximum load time is " << MaximumLoadTimeInS << "s" << endl; From 4b746d329c3d279ba6e697983fd2f00c206e0590 Mon Sep 17 00:00:00 2001 From: Ajay Seth Date: Tue, 17 Jul 2018 14:16:31 -0700 Subject: [PATCH 4/5] Use relative size of data files to justify the scaling of 1000x using the smaller test.sto file as the benchmark. --- OpenSim/Common/Test/testC3DFileAdapter.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/OpenSim/Common/Test/testC3DFileAdapter.cpp b/OpenSim/Common/Test/testC3DFileAdapter.cpp index bbbad1d3c6..9e805bbaee 100644 --- a/OpenSim/Common/Test/testC3DFileAdapter.cpp +++ b/OpenSim/Common/Test/testC3DFileAdapter.cpp @@ -198,16 +198,16 @@ int main() { using namespace std::chrono; steady_clock::time_point startTime = steady_clock::now(); - Storage("std_walking2_grfs.sto"); + Storage("test.sto"); steady_clock::time_point t2 = steady_clock::now(); double loadTime = duration_cast>(t2 - startTime).count(); - // The walking C3D files included in this test should not take more - // than 10x the time to load the 'std_walking2_grfs.sto' Storage on most - // hardware. We make the max time 50x to account for other potential - // slowdowns on CI machines. - MaximumLoadTimeInS = 50 * loadTime; + // The walking C3D files included in this test are benchmarked against the + // load time of a standard 'test.sto' Storage that is 1KB in size. The C3D + // are ~1000x larger than the storage. We make the max time 1200x to account + // for other potential slowdowns on CI machines. + MaximumLoadTimeInS = 1000 * loadTime; cout << "Platform specific Maximum load time is " << MaximumLoadTimeInS << "s" << endl; From c18fb6ecafc6d6ce84cfaf5844fe3a81eed2674b Mon Sep 17 00:00:00 2001 From: Ajay Seth Date: Tue, 17 Jul 2018 14:27:49 -0700 Subject: [PATCH 5/5] Improve comments for the selection of the benchmark and relative scaling. --- OpenSim/Common/Test/testC3DFileAdapter.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/OpenSim/Common/Test/testC3DFileAdapter.cpp b/OpenSim/Common/Test/testC3DFileAdapter.cpp index 9e805bbaee..8dc15d0374 100644 --- a/OpenSim/Common/Test/testC3DFileAdapter.cpp +++ b/OpenSim/Common/Test/testC3DFileAdapter.cpp @@ -201,15 +201,16 @@ int main() { Storage("test.sto"); steady_clock::time_point t2 = steady_clock::now(); - double loadTime = duration_cast>(t2 - startTime).count(); + auto benchLoadTime = duration_cast>(t2 - startTime).count(); // The walking C3D files included in this test are benchmarked against the - // load time of a standard 'test.sto' Storage that is 1KB in size. The C3D - // are ~1000x larger than the storage. We make the max time 1200x to account - // for other potential slowdowns on CI machines. - MaximumLoadTimeInS = 1000 * loadTime; + // load time for a standard 'test.sto' Storage that is 1KB in size. The load + // time is platform specific (e.g. CI machines). The C3D files are ~1000x + // larger than the benchmark file. Therefore, we make the maximum allowable + // load time 1000x the load time of the benchmark file. + MaximumLoadTimeInS = 1000 * benchLoadTime; - cout << "Platform specific Maximum load time is " + cout << "Platform-specific Maximum allowable load time is " << MaximumLoadTimeInS << "s" << endl; std::vector filenames{};