-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alignment make share fix #1043
Alignment make share fix #1043
Conversation
… that requires specific alignment
Nice fix, @bstastnyleapmotion. Obviously not a showstopper, but does using new here instead of make_shared cause an extra copy to happen? I recall we had a similar tradeoff to make between push_back and emplace_back. |
Yeah it has an extra copy but the compiler should optimize it to minimize the cost. (hopefully) |
Bummer given Autowiring doesn't have the best record on getting these things right with performance. Let's have @gittyupagain advise on this as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're talking about copy on construction cost, I'm fairly certain that this has the same number of copies as before. You could use brace initialization if you're concerned about it though.
@gittyupagain profiled and didn't find any noticeable increase in running time. Correct me if I'm wrong but it seems that changing to brace initialization would only have stylistic effect. As for why we didn't encounter the crash before, I believe this was just due to not directly using any Eigen members as autofilter types before:
|
@jdonald