-
Notifications
You must be signed in to change notification settings - Fork 71
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
storage_adaptor<std::vector> missing efficient move assignment from std::vector #338
Comments
you could of course do |
For what exactly do you need unsafe access to the storage? What kind of tasks do you want to do which are not possible with the public API? Your code as written makes no sense, especially from the performance point of view, but also since since the size of the vector does not match with the histogram. The first line is allocating a vector already. The third line is allocating another vector. Even if we supported move assignment, this would still delete the original vector. Allocations are costly, so you want to avoid that. If you need to reset the storage at some point, you can do
or
This copy instruction requires that you know about underflow and overflow bins, if your histogram has them. Safer code is
which skips over underflow and overflow bins. |
The code from the example is just there to show when the call to the The actual task is expanding the bounds to fit the desired values. Our 1d histogram axis doesn't have We actually implemented it in two ways (in both cases we handle over/underflow bins ourselves):
The second option turned out to be more performant so that's the one we chose. |
Ok, thanks for the additional detail. Have you tried to use |
Often times to extend the behaviour of
boost::histogram
, we need unsafe access to storage. And there is one performance issue with working with storage_adaptor.My use case can be simplified to:
The
storage = std::vector<std::size_t>(1000);
line would not call the move assignment operator, but rathervector_impl::operator=(const U& u)
, which copies the values instead of just swapping the data pointer.The text was updated successfully, but these errors were encountered: