7

Explication

Hello, I want to create a function that can execute any type of function, indicating at the end of its execution the time it took. The called function can have a return value or not and 0 or more parameters of any type.

The calling function must print something like this:

Running "myFunction" .....  
Done ! (5210ms)

Basically I want to create a function that calls any type of function passed as a parameter by adding code before and after the call.

What I have done

For now I do it like this.

The calling function:

template <typename T>
T callFunctionPrintTime(std::string fnName, std::function<T()> fn) {
    std::cout << ">> Running " << fnName << " ... " << std::endl;
    auto t1 = std::chrono::high_resolution_clock::now();

    //Call to the target function
    T retVal = fn();

    auto t2 = std::chrono::high_resolution_clock::now();
    auto duration = std::chrono::duration_cast<std::chrono::milliseconds>(t2 - t1).count();

    std::cout << "Done ! (" << duration << " ms)" << std::endl;

    return retVal;
}

The main

int main()
{
    //Store the function to call
    std::function<unsigned long()> fn = []() {
        return myFunction(15, 10000);
    };

    //Use of the function we are interested in
    auto i = callFunctionPrintTime("myFunction", fn);

    //The return value of myFunction can be used in the rest of the program.
    std::cout << "i: " << i << std::endl;
}

myFunction
This function doesn't matter, it can be anything.

Here we execute a while loop for a given maximum time or maximum number of loop and retrieve the number of loop performed.

unsigned long myFunction(long maxMs, unsigned long maxI) {
    unsigned long i = 0;
    auto tStart = std::chrono::high_resolution_clock::now();
    while (maxMs > (std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::high_resolution_clock::now() - tStart).count()) &&
        maxI > i) {
        i++;
    }
    return i;
}

Question

What is the best way for you to do this? I am not satisfied with my code.
I'm not sure I'm using the right way to pass a function of any kind by parameter.
Moreover by using a lambda expression to store my function I can't retrieve the name of the called function. So I have to pass its name by parameter.

  • 1
    If the code already work as you expected, [codereview.se] might be more suitable. Make sure that red read their help center before asking, however – user202729 Jan 20 '21 at 14:21
  • You can never retrieve the name of a function, so that's not really a drawback. – molbdnilo Jan 20 '21 at 14:22
  • 2
    names of functions are not accesible at runtime. You need to resort to either macro voodoo or something like you wrote. Your code looks ok – 463035818_is_not_an_ai Jan 20 '21 at 14:24
  • 1
    That code looks like all "function-measuring" code I've seen. You could possibly avoid the lambda wrapper with some variadic template shenanigans, but that would just make it less general and harder to maintain. – molbdnilo Jan 20 '21 at 14:32
  • 2
    There's no reason to restrict one to a `std::function`. Rather, the callable object can be anything. Then, there's no reason to restrict yourself to functions with no parameters. The template can use a variadic parameter pack for the parameters, that get forwarded to the function. Finally, a function returning `void` won't work like this, so the timing should be done using a helper object that gets constructed in auto scope, and the template is nothing more than instantating the helper object, then `return `, with all the work done in helper's constructor and destructor. The End. – Sam Varshavchik Jan 20 '21 at 14:33
  • Wouldn't it be more convenient to put a simple object at the beginning of the function whose execution time you want to measure, which would store the time points at its constructor and destructor, and print the duration time between the two in the destructor? – zkoza Jan 20 '21 at 14:46
  • 2
    @zkoza That intrudes upon the called function, which might not be editable, and which _shouldn't_ be edited just for benchmarking/testing/whatever in any case. It is correct to measure the time at the caller's side. I grant that it's a nice wee clever use of RAII though! – underscore_d Jan 20 '21 at 15:28
  • There's already an answer [here](https://stackoverflow.com/a/33900479) (very comparable to accepted answer). Kind of a dupe, so voted – JHBonarius Jan 20 '21 at 15:38
  • @JHBonarius OP knows how to measure time. I don't think that's a good fit as a dupe. – Ted Lyngmo Jan 20 '21 at 15:41
  • 1
    @TedLyngmo but the function your proposed answer is generally the same as in an [answer to that question](https://stackoverflow.com/a/33900479)... (Although your coding style is better) – JHBonarius Jan 20 '21 at 15:46
  • @JHBonarius That answer is indeed similar to mine (it took time to read them all through) :-) - It ignores the return value of the function though. Perhaps a minor detail but for functions with `[[nodiscard]]` it'll be bad. ...and thanks :-) – Ted Lyngmo Jan 20 '21 at 15:50
  • @underscore_d Yes, these are good points. I was thinking of a scenario where a function is called several times with different arguments (e.g. containers of various sizes) and its execution time may vary considerably from call to call. Then it is difficult and time-consuming to mimic this in an artificial environment, and a possibility of "injecting" a timer in live code is an advantage in terms of information gained to the work necessary to obtain it. – zkoza Jan 20 '21 at 15:57

2 Answers2

4

I'm pretty sure there's no single answer to what's best - but this is a small improvement i.m.o. since it's a bit more generic.

#include <chrono>
#include <iostream>
#include <string>
#include <type_traits>

// enable it for invocables with any type of arguments
template <class Func, class... Args,
          std::enable_if_t<std::is_invocable_v<Func, Args...>, int> = 0>
decltype(auto) callFunctionPrintTime(std::string fnName, Func fn, Args&&... args)
{
    std::cout << ">> Running " << fnName << " ... " << std::endl;
    auto t1 = std::chrono::high_resolution_clock::now();

    //Call to the target function by forwarding the arguments to it
    decltype(auto) retVal = fn(std::forward<Args>(args)...);

    auto t2 = std::chrono::high_resolution_clock::now();
    auto duration = 
        std::chrono::duration_cast<std::chrono::milliseconds>(t2 - t1).count();

    std::cout << "Done ! (" << duration << " ms)" << std::endl;

    return retVal;
}

Alternatively, if you don't plan on making overloads for non-invocables (which seems pretty obvious that you wont when I think about it) you can use static_assert instead of SFINAE:

template <class Func, class... Args>
decltype(auto) callFunctionPrintTime(std::string fnName, Func fn, Args&&... args)
{
    static_assert(std::is_invocable_v<Func, Args...>, "must be invocable");
    //...

Test usage:

int& a_func(int i) {
    static int rv = 0;
    rv += i;
    return rv;
}


int main() {
    int& ref = callFunctionPrintTime("a_func 1", a_func, 10);
    
    std::cout << ref << '\n';  // prints 10
    
    ref += 20;

    callFunctionPrintTime("a_func 2", a_func, 100);

    std::cout << ref << '\n';  // prints 130 (10 + 20 + 100)
}

Or some of the alternatives for calling myFunction:

std::function<unsigned long()> fn = []() { return myFunction(15, 100000); };

std::cout << callFunctionPrintTime("myFunction", fn);
std::cout << callFunctionPrintTime("myFunction",
                                   []() { return myFunction(15, 100000); });
std::cout << callFunctionPrintTime("myFunction", myFunction, 15, 100000);

Some useful links: decltype(auto), std::enable_if_t, std::is_invocable_v, SFINAE

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • 1
    Yeah, templates are preferable to `std::function` generally unless the latter's type erasure is really needed or templates are proven to be some kind of bottleneck (unlikely in most cases) – underscore_d Jan 20 '21 at 15:13
  • @underscore_d Yes and here one still has the option to use `std::function` if needed. – Ted Lyngmo Jan 20 '21 at 15:16
  • 1
    Tks you for the answer! It answers my problem and moreover the function is generic. I don't know `decltype(auto)`, `std::enable_if_t` or `std::is_invocable_v` so I'll go find out. – Maxime Charrière Jan 20 '21 at 15:28
  • @CharrièreMaxime You're welcome! I added some links to the answer too. – Ted Lyngmo Jan 20 '21 at 15:34
3

Main idea is correct. there are some details which might be improved:

template <typename Func, typename ... Ts>
decltype(auto) callFunctionPrintTime(std::string_view fnName, Func&& f, Ts&&... args) {
    static_assert(std::is_invocable_v<Func&&, Ts&&...>); // Possibly SFINAE instead.
    std::cout << ">> Running " << fnName << " ... " << std::endl;

    struct Finally {
        std::chrono::time_point<std::chrono::high_resolution_clock> t1 =
            std::chrono::high_resolution_clock::now();

        ~Finally() {
            auto t2 = std::chrono::high_resolution_clock::now();
            auto duration =
                std::chrono::duration_cast<std::chrono::milliseconds>(t2 - t1).count();

            std::cout << "Done ! (" << duration << " ms)" << std::endl;
        }
    } finally;

    return std::invoke(std::forward<Func>(f), std::forward<Ts>(args)...);
}

Now:

  • handles void return type (without specialization required).
  • Log also in case of exception (You can go further with std::uncaught_exceptions or try/catch block to dissociate exception from normal path).
  • handle any invocable with its parameters.

For automatic name, we have to rely on MACRO:

#define CallFunctionPrintTime(F, ...) callFunctionPrintTime(#F, F __VA_OPT__(,) __VA_ARGS__)

Demo

Jarod42
  • 203,559
  • 14
  • 181
  • 302