Computing function time in C++

Question

I used the std::chrono library for computing the timing of a function and need some review on it.

Here is the code:

namespace gstd
{
#ifdef GTIME
 inline namespace gtime
 {
 template<typename type, typename period>
 using duration = std::chrono::duration<type, period>;
 std::function<std::chrono::time_point<std::chrono::steady_clock>( )>current_time = std::chrono::high_resolution_clock::now;
 template<typename RetType>
 std::optional<double> getFunctionTime(std::function<RetType(void)>function) {
 try { 
 auto start_time = current_time( );
 function( );
 auto end_time = current_time( );
 duration<double, std::milli> dur = end_time - start_time;
 return dur.count( );
 }
 catch ( ... ) {
 return {}; 
 }
 }
 template <typename RetType, typename arg>
 std::optional<double> getFunctionTime1(std::function<RetType(arg)> function, arg value) {
 try {
 auto start_time = current_time( );
 function(value);
 auto end_time = current_time( );
 duration<double, std::milli> dur = end_time - start_time;
 return dur.count( );
 }
 catch ( ... ) {
 return {};
 }
 }
 template<typename RetType, typename arg1, typename arg2>
 std::optional<double> getFunction2(std::function<RetType(arg1, arg2)>function, arg1 firstparam, arg2 secondparam) {
 try { 
 auto start_time = current_time( );
 function(firstparam, secondparam);
 auto end_time = current_time( );
 duration<double, std::milli> dur = end_time - start_time;
 return dur.count( );
 }
 catch ( ... ) {
 return {};
 }
 }
 }
#endif // GTIME 
}

This is the code in the header file. Now how i am using it is here:

#define GTIME
#include "GStandard.h"
#include <iostream>
int number_crunching( ) {
 int sum { 0 };
 for ( int i = 0; i < 10000000; i++ )
 sum += 1;
 return sum;
}
int multiplier(int number) {
 for ( int i = 0; i < 10000000; i++ ) {
 number *= ( i + 1 ); 
 }
 return number;
}
int sum( int a, int b) {
 return a + b;
}
int main( ) {
 auto ftime = gstd::getFunctionTime<int>(number_crunching);
 auto f1time = gstd::getFunctionTime1<int, int>(multiplier, 5);
 auto f2time = gstd::getFunction2<int, int, int>(sum, 4, 5);
 std::cout << ftime.value( ) << '\n';
 std::cout << f1time.value( ) << '\n';
 std::cout << f2time.value( ) << '\n';
 std::cin.get( );
 return 0;
}

Some suggestions were previously made on making it variadic but I am not doing it so that the students at my college can make improvement to this code. I am looking forward to removing the try catch block and if there are any more changes that you guys could suggest that would be very nice.

Answer 1

Be more generic

As I've suggested here, you should not force the use of an std::function. Instead, take any type of callable and either a fixed or an unlimited number of arguments, using std::invoke inside your implementation.

More importantly, you really don't need multiple functions for a different number of arguments; use variadic templates, e.g. have a signature such as:

template< class F, class... Args>
std::optional<double> getFunctionTime(F&& f, Args&&... args) noexcept;

(ignoring any other potential changes)

This will make it easier for you to invoke, too:

auto ftime = gstd::getFunctionTime(number_crunching);
auto f1time = gstd::getFunctionTime(multiplier, 5);
auto f2time = gstd::getFunctionTime(sum, 4, 5);

Alternatively, you could only accept 0-argument runnables, in which case you simply pass lambdas:

auto f1time = gstd::getFunctionTime([]() { return multiplier(5); });

Return a chrono::duration

Don't convert your duration to a plain numeric value - keep the more specific type.

Use integral values for durations

Why use a floating-point value in the duration? Stick with integral values unless you have a good reason to do otherwise.

Consider changing the name

I'd use time_an_invocation or time_invocation but I guess that's a matter of taste.

Consider leaving it to the caller to handle exceptions

You're forcing a certain behavior w.r.t. exceptions. I'd consider simply not handling them, so that timing the invocation of a function that throws an exception, throws an exception. That would simplify your code and avoid a dependence on std::optional, which the caller might not be interested in. Remember the caller has to have additional code to handle the empty-optional case anyway.

Consider using a stopwatch class instead

In a related question, @Edward suggests keeping the invocation and the time measurement separate, using a "stop-watch" class. That might not be "better" but it's certainly worth considering.