Remove inefficiencies created by introducing specific C++11 #503
Conversation
|
The copy constructor and copy assignment operator are required since the The move constructor and assignment operator was from a specific (private) user request. Let me check with them to see if a copy instead is enough. |
|
I think that having a move constructor and assignment operator is a good idea as long as it is implemented correctly. In the current implementation, it simply copies the data members. We should be setting the "right side" of the operation to a state in such a way that it was default constructed or some other empty but valid state. |
|
@skizzay I would rather keep the move constructor/assignment and set the arg (move from) to a default constructor state ( |
|
@tmontgomery I apologize for the late response. I've incorporated the changes you've suggested. For moved objects, I call |
|
No worries. Thanks for the PR! |
|
These changes broke the build. Specifically, I think the constructors for fixed flyweights aren't being handled appropriately. |
2 participants
As SBE generates C++ code, it's doing more than it needs to do and in some cases, actually hurting performance.
First, SBE is generating a move constructor and move assignment operator for the generated class. However, the implementation of these methods are the exact same as their copy counter parts (see note below). Since T&& will be promoted to const T&, the copying methods are sufficient.
Next, the C++03 version of the class member template function forEach takes a single template parameter which is meant to be a functor. The signature has changed in C++11 version to take in a std::function. This is where the inefficiency is introduced. The implementation of std::function uses type-erasure to achieve its functionality. However, that means it's performing an allocation on the heap, copy the data of the functor, and invokes the functor indirectly which loses the ability to inline the method call. By leaving it as a template parameter, the compiler has the opportunity to inline the invocation altogether and allows the compiler to inline the invocation.
NOTE: We can take this a step further and declare that the copy constructor and copy assignment operator methods are not necessary since it's the exact same implementation that the compiler would generate for us anyway.