Open DanRStevens opened 2 years ago
One area of concern I wanted to mention regarding the use of std::function
, is there are some reports of performance issues with std::function
, as compared to using lambas with templated methods. In particular, it adds a virtual function call, and may result in allocation of external memory when storing certain large functions, such as lambdas with a large capture group. These are not likely to be of any concern to our particular use case. In particular, for user input handlers, the tiny amount of overhead is never going to matter. Nevertheless, it might be good to better understand the situation by learning how std::function
works.
First off, the performance comparison was relating std::function
to template methods taking a lambda, which could inline the lambda call when instantiating the template method. The current implementation of Delegate
doesn't do that. It uses pointers internally, and so it's already very similar to the virtual function call dispatch offered by std::function
. The main concern then is the potential allocation of external memory.
Second, the potential use of external memory is because std::function
takes ownership of the function it holds, and it can hold arbitrarily large functors, such as lambdas with large capture groups. The current implementation of Delegate
doesn't provide for capturing anything beyond the this
pointer, and a member function pointer, so we effectively already limit capture group size. Further, std::function
offers a SBO (Small Buffer Optimization), which is generally sufficient to store such references without allocating external memory. Effectively, the only cases where std::function
should need to allocate, are cases not currently supported by the Delegate
code.
According to the C++ standard, the std::function
object will not allocate external memory when used to store two particular small cases:
std::reference_wrapper
to a callable objectNotably absent is a pointer to a member function. This is relevant, as all our current uses are pretty similar to calling std::bind
on a this
pointer with a member function pointer. As can be demonstrated, using std::bind
to group a this
pointer with a member function pointer does generally produce a functor that is too large to store in a std::function
without externally allocated memory. However, the solution to that is simply to use a lambda rather than std::bind
, as the lambda generally does fit without the SBO constraints offered by std::function
.
Example:
#include <iostream>
#include <functional>
struct Object{
void method() {}
};
int main() {
Object object;
const auto func1 = std::bind(&Object::method, &object);
const auto func2 = [&object]{ object.method(); };
std::cout << sizeof(func1) << std::endl; // 24 - (64-bit g++)
std::cout << sizeof(func2) << std::endl; // 8 - (64-bit g++)
std::function<void()> callback;
callback = func1; // external allocation
callback = func2; // no external allocation
}
The reason for the difference in size is the lambda stores only a single pointer to object
, where as the std::bind
call produces an object with a pointer to object
, and a somewhat large member function pointer, which consists of a function pointer and an offset value for the this
pointer. For the lambda, that member function pointer and this
adjustment is effectively compiled into a thunk when the compiler generates code for the lambda's operator()
, and so become part of the code rather than extra data fields.
As for verifying the lambda can be stored without extra memory allocation, that can be done with valgrind
, or with other test code, such as is presented in one of the ansers to this StackOverflow question:
Avoid memory allocation with std::function and member function
In summary, std::function
can replace all our current uses of Delegate
with no loss of efficiency, plus support other new uses of lambdas, which may (or may not) incur a slight overhead over the existing code if they are actually used (and only when they are used).
On a related matter, there is a proposal for a non-owning function_ref
that never requires external storage. Some reasonably short code (~200 line) can be found on GitHub:
It may be worth giving some consideration to ownership characteristics. Should Signal
store owning references or non-owning references to delegates.
Currently SignalSource
stores the Delegate
instances internally, and makes calls to them at arbitrary later times. This seems to imply ownership, and so it would make sense for SignalSource
to use the characteristics of std::function
.
On the other hand, SignalSource
offers a disconnect
function, which requires passing in a Delegate
instance, which can then be searched for and removed from the list of connections. This implies the Delegate
instance may exist externally for the duration of it's active lifetime. This would imply that function_ref
may have suitable characteristics.
Somewhat complicating the matter is that all Delegates
are cleaned up when SignalSource
is destructed, and so automatically cleaned up Delegate
instances might not have any corresponding external instances. Additionally, the current Delegate
class has a trivial destructor, so there really is no cleanup, and no particular relevance to ownership characteristics. In fact, during cleaning, sometimes a new Delegate
object is constructed, which is merely compared as having equal value to the stored Delegate
instance (corresponding fields are equal), rather than having the same identity (same address).
When the delegate is a callback to a long lived object, extra storage can be kept in that long lived object, rather than the delegate. Such a setup makes std::function
and std::function_ref
somewhat equivalent. Instead of capturing lots of values and copying them into a lambda, a reference can be stored to externally managed data. This is pretty much the example from the earlier posts where the delegates captures &object
.
For more local usage, such as for an immediate callback, catpure group parameters could potentially be stored in a struct
or std::tuple
, which is captured by reference:
auto data = std::tuple(a, b, c);
auto func = [&data]() { doSomething(data); };
someFunctionCallWithACallback(func);
This seems to be more what function_ref
is about, since here the lambda doesn't need to live past the end of the function call it was passed to. For such a case, the function could take a function_ref
instead of a std::function
, which would allow it to bind to a lambda with a larger capture group without external memory allocation:
someFunctionCallWithACallback([&a, &b, &c]() { doSomething(a, b, c); });
This isn't really our use case though.
I think we should probably stick with std::function
, rather than try to force function_ref
hoping to gain efficiencies in areas we don't actually need or could easily work around.
A deeper question is if we even need the Signal
abstraction. The point of SignalSource
is to distribute an event to multiple listeners. The current code base only appears to ever have a single listener for any event. We could be using Delegate
directly, rather than using SignalSource
. Though I suppose there is that "what if" scenario.
I spent a fair bit of time doing research into std::function
, and background material on "Type Erasure". I thought maybe I should store a list of links for posterity.
Basic research:
std::function
and member functionAndrzej's C++ blog:
The Old New Thing:
Arthur O'Dwyer's blog:
Misc related research:
std::ref
and std::reference_wrapper
: common use casesStandard library documentation:
Standard library implementations and proposals:
So I've discovered a bit of a snag. There is no operator==
on std::function
, which means you can't search a collection for an instance of one, either to check for duplicates in connect
, or to remove an existing entry in disconnect
.
There is a target
template method, which can convert the std::function
to the underlying function type, and which could potentially be used to write an equality comparison. However, if you're converting to a lambda type, the lambda also lacks operator==
.
Conceptually it should be fairly simple to compare a lambda instance if you know they're the same type, and all captures are equality comparable. It would essentially be the same as the default equality comparison for any regular struct
. Though given the nature of lambdas, I don't know of any way to add such a comparison operator to one.
Bit of a side note, but I've always found the DefaultVoidToVoid
template in the Delegate
code to be a bit unexpected. It doesn't appear to actually do anything. I assume it was some kind of workaround for an earlier compiler bug, though I've never found any document that describes that in any sort of detail.
Edit: Found this: https://github.com/dreamcat4/FastDelegate/blob/master/FastDelegate.h
// DefaultVoid - a workaround for 'void' templates in VC6. // // (1) VC6 and earlier do not allow 'void' as a default template argument. // (2) They also doesn't allow you to return 'void' from a function. // // Workaround for (1): Declare a dummy type 'DefaultVoid' which we use // when we'd like to use 'void'. We convert it into 'void' and back // using the templates DefaultVoidToVoid<> and VoidToDefaultVoid<>. // Workaround for (2): On VC6, the code for calling a void function is // identical to the code for calling a non-void function in which the // return value is never used, provided the return value is returned // in the EAX register, rather than on the stack. // This is true for most fundamental types such as int, enum, void . // Const void is the safest option since it doesn't participate // in any automatic conversions. But on a 16-bit compiler it might // cause extra code to be generated, so we disable it for all compilers // except for VC6 (and VC5).
ifdef FASTDLGT_VC6
// VC6 workaround typedef const void * DefaultVoid;
else
// On any other compiler, just use a normal void. typedef void DefaultVoid;
endif
So it looks like it's a workaround for VC6. That's pretty old, and rather irrelevant at this point.
Edit: Found a lot of comments describing why things are they way they are were removed back in: 7a92544e4a0424d06b48685957f2b81c1bffdbe7
Our
Delegate
code is super old. We should consider updating it.The
Delegate
code has a lot of macro#if
to support really old compilers. Our code relies on C++17, and perhaps soon, C++20. That means we need a reasonably modern compiler. We could gut many of the#if
checks for old compiler support from theDelegate
code.Our custom
Delegate
implementation is from about 2005. Source: https://www.codeproject.com/Articles/11015/The-Impossibly-Fast-C-DelegatesThe current implementation far predates the C++11 standard, which introduced
std::function
. It seemsstd::function
replaces the need to write a customDelegate
class. Maybe we should update to usingstd::function
and get rid of the customDelegate
class.Our current implementation of
Delegate
andSignal
only really supports member function pointers. We should add lambda support when callingSignal::connect
andSignal::disconnect
.It may be possible to inline the lambda. Here's how lambdas would compare to the existing way:
If we move to
std::function
, it may make sense to use lambdas as the primary means of representingDelegate
. That might mean using it as the primary storage type internally, or perhaps updating calls to prefer passing lambdas.