4
\$\begingroup\$

I was trying to implement a faster, alternative implementation of std::function. I came up with the code below:

template <typename R, typename ...Args>
struct Function {
    template <typename Lambda>
    Function(Lambda f) {
    static auto function = f;
    func = + [] (Args... args) -> R {
        return function(args...);
    };
    };
    Function(const Function& other) : func(other.func) {}
    inline auto operator() (Args... args) {
        return func(args...);
    }
    R (*func) (Args... args);
};

Now you can instantiate a Function like below:

int y = 100;
int x = 56;
auto f1 = Function<int , int>([&] (int z) {return x + y + z});
// If you need to store a generic custom functor class:
auto functor = SomeCustomFunctorClass<int, int>(); // Assuming functor takes an int as an argument and returns an int
auto f2 = Function<int , int>([functor] (int x) {return functor(x);});

Note that my implementation Function can only store lambdas, but storing a generic lambda can be achieved through the example above.

The idea here is to erase the type of the lambda in the constructor by storing it statically, which means that I can use it the function pointer func. Note that, a different static variable is instantiated each time the constructor is called because lambdas all have a unique type, so each time a new version of the constructor is instantiated with a different constructor.

I've benchmarked this implementation, and it runs significantly faster than std::function (with optimisations enabled). It is up to 6 - 7 times faster for construction, 15 times faster for copying, and calling it is as fast as calling a function pointer (though significantly slower than calling a lambda which is always inlined).

However, this almost seems too easy, with this implementation being only 15 lines of code. Are there any drawbacks to my implementation compared to the traditional implementation of std::function using heap allocation and virtual calls? Perhaps statically allocating the lambda is not a good idea?

\$\endgroup\$

1 Answer 1

7
\$\begingroup\$

Questions & answers

“Are there any drawbacks to my implementation compared to the traditional implementation of std::function using heap allocation and virtual calls? Perhaps statically allocating the lambda is not a good idea?”

Yes, definitely a bad idea, with numerous drawbacks.

Because there are no compile-time checks to ensure that you’re actually using a lambda, this type is very brittle and very dangerous. Today I may write code like this:

auto x = 0;
auto y = 0;

auto func_x = [&x] { ++(*x); };
auto func_y = [&y] { ++(*y); };

// ... later:
auto function_x = Function<void>{func_x};
auto function_y = Function<void>{func_y};

No problems there. (Well, I mean, yes, problems, but not for this specific presumed use case. But we’ll get back to that.)

But then, later in the development process, someone refactors those two lambdas into this:

struct indirect_incrementer
{
    int* p_val = nullptr;

    auto operator()() { ++(*p_val); }
};

// then the following two lines:
//auto func_x = [&x] { ++(*x); };
//auto func_y = [&y] { ++(*y); };
// become:
auto func_x = indirect_incrementer{&x};
auto func_y = indirect_incrementer{&y};

It’s an innocent and perfectly logical refactor… but now everything blows up, because your assumption that every function object type Function is constructed with no longer holds. The static variable function is initialized with a copy of func_x, and so calling function_y does not call func_y, it calls func_x.

This isn’t just a problem with using function objects. Even if you somehow completely banned the use of function objects with Function—like, say, with some kind of linter or something—and made 100% sure that Function is always called with a legit lambda object, the problem still exists. Because despite your thinking, it is possible for a lambda type to be non-unique. It’s even quite trivial to do… just call the function that contains the lambda more than once:

auto render()
{
    // ... [snip] ...

    // somewhere in your render function, you use Function:
    auto func = Function<int, int>{[&] (int i) { return ++i; }};

    // ... [snip] ...
}

// elsewhere, in your main game loop:
while (not done)
{
    input();
    update();
    render(); // <- !!!
}

render() is called in a loop, but only the first loop will initialize the static function variable. Meaning every other time you use func, it will be with dangling references to the locals in that first render() call.

Now you could “fix” that problem with various hacks, basically detecting whether function was previously initialized, or counting the number of times it was initialized, or something like that. But it won’t fix the other problems.

The other big problem with using a static variable to hold a copy of the lambda is that you’re quietly cheating C++’s scope rules. This could lead to frustrating and nasty surprises. For example:

auto p_weak = std::weak_ptr<int>{};

// in some more restricted scope:
{
    auto p = std::make_shared<int>();
    p_weak = p;

    auto func = Function<void>{[p] { if (p) ++(*p); }};

    // use func somehow

    // scope is ending, so func and p are being destroyed, right?
    //
    // ... *right*?
}

if (auto p = p_weak.lock(); p)
    std::cerr << "memory leak!";

Turns out, p is never actually destructed; the lambda takes a copy, which is then copied into the static function variable, which then holds on to it… forever (well, until after main() returns, at least). This is not just a shared_ptr problem; I just used shared_ptr to be able to access otherwise lost memory. Any value that gets captured by the lambda is never destructed during the lifetime of the program. In other words, the more you use Function—even when every use avoids the other problems mentioned—the more you leak memory.

That’s probably why you’re seeing such large increases in speed: the function object is only being copy-constructed once—even when that’s incorrect—and it’s never being destructed.

(Also, std::function has a ton of very important and useful other abilities and benefits Function doesn’t offer, like the ability to be null-constructed, and reseated. Mere type-erasure is useful, I suppose, but probably not useful enough on its own… especially when simple function pointers can do that (and more!).)

So yes, using a static variable to get around dynamic allocation is not a good idea.

Code review

template <typename Lambda>
Function(Lambda f) {
static auto function = f;
func = + [] (Args... args) -> R {
    return function(args...);
};
};

Proper indentation would definitely help make this more readable. It would also highlight the stray semicolon you have at the end.

The fact that you’re copying f into function makes the whole class unusable with move-only lambda types:

auto p = std::make_unique<int>();

auto lambda_p = [p = std::move(p)] { if (p) { *p = 42; } };

auto func_p = Function<void>{lambda_p}; // won't compile

// (also, as mentioned previously, p won't ever be freed)

You should also use forwarding references in all the function calls. Right now every function argument will be taken by-value, which will be at least slow, and probably won’t compile in a lot of cases.

Function(const Function& other) : func(other.func) {}

This is entirely unnecessary.

inline auto operator() (Args... args) {
    return func(args...);
}

The inline keyword does nothing here.

Again, you should really be using forwarding references.

The whole interface of Function is also rather clunky. It seems specious to have to give the return and argument types, when they’re obvious and deduce-able from the lambda itself. It would be much nicer to be able to write:

auto f1 = Function{[&] (int z) {return x + y + z}};
// f1 is deduced as Function<int, int>

This is a situation where deduction guides would be useful.

Summary

You can’t get away with avoiding having function object wrappers actually wrap the function objects. Put that way, it should seem pretty obvious.

Your type-erasing wrapper either needs to dynamically allocate the memory to copy a function object, or use the small object optimization, or both. Trying to hide the wrapped object in a static variable is cheating. You may get away with it for a little while… but you will eventually be caught, either via memory leaks, reading/writing dangling references, or simply reading/writing to the wrong objects.

\$\endgroup\$
6
  • 1
    \$\begingroup\$ Awesome counter example with weak_ptr. I wouldn’t want to be one searching for that memory leak. \$\endgroup\$ Jan 24, 2021 at 19:53
  • \$\begingroup\$ Thank you for taking the time to review my code. I had never thought about the loop giving a non-unique lambda each time in your game loop example. Concerning the memory leaks via static allocation, how does it compare to say, string literals being statically allocated in c++? \$\endgroup\$ Jan 24, 2021 at 21:03
  • \$\begingroup\$ Just fixed the Function2 typo. Function2 was the original name of this struct (this was my second attempt), but when i copy pasted on code review I took of the '2' off the class name but forgot to take it off the constructor. Apologies. Should compile cleanly now. \$\endgroup\$ Jan 24, 2021 at 21:07
  • \$\begingroup\$ That depends on whether you mean a string literal or a std::string literal. Regardless, the general rule (ignoring optimizations) is that every static variable takes up memory for the entire duration of the program even if it’s never initialized. This really isn’t a big deal unless you’re working in really constrained environments, like tiny embedded stuff, because we’re talking tiny little bits of memory. sizeof a lambda is generally pretty small. \$\endgroup\$
    – indi
    Jan 29, 2021 at 20:42
  • \$\begingroup\$ The real problem is locking up acquired resources. I just used the memory resources acquired by a shared_ptr to illustrate, but if instead of a shared_ptr, you passed a database handle to that lambda, then you’ve locked up that database for the entirety of your program, with no way to release it, which may prevent you from opening subsequent handles to the database. Or instead of a database handle, it could be file lock. Or whatever. \$\endgroup\$
    – indi
    Jan 29, 2021 at 20:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.