0

I needed a way how to measure the execution time of a function. I found this very good answer on SO https://stackoverflow.com/a/21995693/3179492. It is a perfect solution.

It is using a function pointer with variadic parameters list.

Here is the MCVE:

#include <algorithm>
#include <chrono>
#include <stdio.h>

class Foo
{
public:
    auto foo_member( int x ) -> void { printf( "in foo_member(%d);\n", x ); }
};

class measure
{
public:
    template<typename F, typename ...Args>
    static typename std::chrono::microseconds::rep execution(F func, Args&&... args)
    {
        auto start = std::chrono::system_clock::now();
        func(std::forward<Args>(args)...);
        auto duration = std::chrono::duration_cast<std::chrono::microseconds> 
                                  (std::chrono::system_clock::now() - start);
        return duration.count();
    }
};

int main()
{
        Foo foo;
        int x = 1234;

        // this line does not compile       
        printf( "execution time=%ld\n", measure::execution( foo.foo_member, x ) );
}

This code does not compile because foo_member is not static. The error message is invalid use of non-static member function.

There are few ways how to resolve the problem. For me, the most elegant, shortest and effective way is this:

printf("execution time=%ld\n", measure::execution( [&foo, &x] () { foo.foo_member(x); } ));

This means, I use a lambda function in order to get the line compiled.

I am just wondering why the compiler could not do it for me? The code path is exactly defined how to translate the first version into a lambda function using the capture mechanism. When you just realize what a modern c++ compiler is doing with the code this would be indeed one of the simplest code reordering...

Note:

It is compiled with these gcc flags: -Wall -Werror -Wextra -std=c++11

Community
  • 1
  • 1
Peter VARGA
  • 4,780
  • 3
  • 39
  • 75
  • 2
    Why would you expect the compiler to generate this automatically for you? Another way you can achieve this is simply using `std::bind()`. – πάντα ῥεῖ Jul 11 '16 at 18:58
  • Some questions that would immediately come up if this were allowed: what would the type of `foo.foo_member` be? Would it be some compiler-generated type? Would that mean that `foo.foo_member == foo.foo_member` would fail, complaining that the comparison involves two unrelated types? If so, why? Wouldn't it make sense to allow `auto x = y ? foo.a : bar.a;`? How could the feature be redesigned to support that? ...And back to the drawing board. This probably isn't the answer, but it's not as simple as you suggest it is. –  Jul 11 '16 at 19:00
  • @πάνταῥεῖ `std::bind()` needs another include file. The solution with `lambda` uses only c++ code. This means for me, the c++ compiler can rearrange it without additional information [in this case, including the header `functional`] – Peter VARGA Jul 11 '16 at 19:06
  • "why the compiler could not do it for me" Because to support it would require new, most likely ambiguous, syntax for a feature that arguably nobody really needs. `foo.foo_member` doesn't have any meaning in current C++ - specifically, it's not a syntax for a member method pointer. You're arguing that it should be syntax for `std::bind(&Foo::foo_member, foo, {placeholders})`. If so, then you should phrase your question so, and that'd immediately make it obviously off-topic (because it is!!!). – Kuba hasn't forgotten Monica Jul 11 '16 at 20:21

2 Answers2

2

foo_member is a non-static member function, so it takes an implicit first argument, the this pointer, which you must pass to it. The proper syntax for creating a pointer to member function is &ClassName::Func, not class_instance.Func.

To fix your example, make the following changes:

Within main

printf( "execution time=%ld\n", measure::execution( &Foo::foo_member, &foo, x ) );

Now you're passing a pointer to Foo::foo_member, and a pointer to Foo instance as the first argument. foo_member will be invoked on this instance.

measure::execution needs to be modified to handle pointer to member functions correctly. The easiest way to do this it to probably to use std::experimental::apply.

std::experimental::apply(func, std::forward_as_tuple(std::forward<Args>(args)...));

Or if you have a C++17 compiler, you can use std::invoke, in which case you can avoid the call to forward_as_tuple.

Live demo (with std::experimental::apply)

Live demo (with std::invoke)

Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • Your solution also modifies the `execution` function. I tried to point out that only the call - which causes the error message - can be rewritten with c++ code without touching other parts and due to the exact defined code path a compiler could do it with some command line options enabled. – Peter VARGA Jul 11 '16 at 19:14
  • @AlBundy Then use `bind` or a lambda as you've shown. Or write a separate overload of `execution` to handle pointer to member functions. – Praetorian Jul 11 '16 at 19:18
  • I know how to solve the problem. I was just *loud* thinking why the compiler can't do it for me. – Peter VARGA Jul 11 '16 at 19:19
  • @AlBundy I didn't realize this was a *why doesn't feature X exist?* question. The simple answer to that is because no one has proposed it or it has been proposed but got rejected. Currently there is no syntax to bind the this pointer to a member function using the `instance.Func` syntax, you need to form a pointer to member and pass it the instance pointer as the first argument. – Praetorian Jul 11 '16 at 19:25
0

You're arguing for adding completely new syntax to do what can already be done in many ways. The simplest solution is to use mem_fn:

measure::execution(std::mem_fn(&Foo::foo_member), foo, x)

You could also use std::bind or boost::bind:

using std::placeholders;
measure::execution(std::bind(&Foo::foo_member, foo, _1), x)
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313