Opened 15 years ago

Last modified 5 years ago

#1045 new Feature Requests

[multi_array] Need a proper swap operation

Reported by: Marcus Lindblom <macke@…> Owned by: Ronald Garcia
Milestone: To Be Determined Component: multi_array
Version: Boost 1.34.0 Severity: Optimization
Keywords: Cc: rhys.ulerich@…, Maxim.Yanchenko@…

Description

See this email which was unanswered.

Basically, a proper pointer swap operation would be both efficent and useful.

(P.S. multi_array is not listed in components?)

Attachments (1)

multi_array_swap.patch (10.8 KB ) - added by Rhys Ulerich <rhys.ulerich@…> 13 years ago.
Patch implementing the requested functionality

Download all attachments as: .zip

Change History (13)

comment:1 by Ronald Garcia, 15 years ago

Owner: set to Ronald Garcia

comment:2 by Marshall Clow, 15 years ago

Component: Nonemulti_array

comment:3 by Chris Kieschnick <wildjokerman@…>, 13 years ago

I too would benefit from this feature as I have a large boost::multi_array (elements number in the millions or hundreds of thousands) in a thread which I am handing off to another thread before the initial thread dies. (This thread is merely an algorithm and it must post its result to a queue which maintains result output in the same order that each algorithm thread was started). I don't think it should be necessary that I use boost::shared_ptrs for a structure with such a small local footprint (some pointers and other information I presume). For now I do a copy until I can get around to refactoring to add a shared_ptr.

comment:4 by Rhys Ulerich <rhys.ulerich@…>, 13 years ago

Cc: rhys.ulerich@… added

Me too on the feature request. I've got some low-storage numeric algorithms that sit atop multi_array and multi_array_ref. Being able to swap two multi_arrays or two multi_array_refs would be very, very helpful.

by Rhys Ulerich <rhys.ulerich@…>, 13 years ago

Attachment: multi_array_swap.patch added

Patch implementing the requested functionality

comment:5 by Rhys Ulerich <rhys.ulerich@…>, 13 years ago

Attached is a patch against 1.41 providing multi_array_ref::swap and multi_array::swap, overloads for boost::swap, some unit tests and a couple of very minor documentation updates. Tests pass against gcc 4.2.4.

Sorry for not working from trunk; the resize tests were broken there. The patch rewrites a small portion of resize in terms of the new multi_array::swap operation and I wanted the resize test cases to run against that change.

comment:6 by Rhys Ulerich <rhys.ulerich@…>, 13 years ago

For whomever reviews this, some details about the patch that I'm not sure I got correct:

  • Handling of non-default allocators
    • In particular, swapping between instances using different allocators
  • No throw semantics?

comment:7 by rhys.ulerich@…, 12 years ago

Just capturing some old email to/from Ronald Garcia regarding the patch...

Sat, Dec 12, 2009 at 1:01 PM
From: Ronald Garcia <garcia@osl.iu.edu>
To: Rhys Ulerich <rhys.ulerich@gmail.com>
Hi Rhys,

I have looked over your patch to multi_array.   The good news is
that on the whole I like what you are proposing.  The bad news is
that I will want to make some more fundamental changes to MultiArray
before or at the same time as I add this functionality to the library.

The problem right now is that swap as you propose it is not simply
a faster-but-equivalent version of the inefficient operator=() based
implementation of swap. However I see that more as a problem with
operator=() as it's currently presented than a problem with your swap. 
The current implementation of multi_array and the MultiArray concept
use operator=() to represent deep copy semantics, and I now see that
as problematic because of the ways that C++ treats operator=() and
constructors specially.

Before I add swap to multi_array and multi_array_ref, I need to rename
the functionality currently associated with operator=() and remove
operator= from the concept.  Then multi_array and multi_array_ref can
independently support a more traditional operator=() behavior, which
would be compatible with your swap.

I appreciate your contribution to multi_array, and will see about getting
other changes going, but it won't happen before the next release is
frozen I'm afraid.

Best,
Ron
Sat, Dec 12, 2009 at 1:09 PM
From: Rhys Ulerich <rhys.ulerich@gmail.com>
To: Ronald Garcia <garcia@osl.iu.edu>
Hi Ron,

Thanks for looking over the patch, and no problem on holding off on
the functionality.

My $0.02 is that the deep copy semantics associated with operator=
seem reasonable/natural to me in the current implementation, but I may
be missing some nuances in the language.  I'd be interested in seeing
the upcoming changes as they develop.  Do you use the boost
development list for MultiArray discussion?  Or some other list where
I can subscribe?

- Rhys
Sun, Dec 13, 2009 at 8:08 AM
From: Ronald Garcia <garcia@osl.iu.edu>
To: Rhys Ulerich <rhys.ulerich@gmail.com>
Hi Rhys,

Thanks for your feedback.  There hasn't been a great deal of
multi_array discussion in some time, but I will write to the
boost developers list about these issues.

Cheers,
Ron

comment:8 by Maxim Yanchenko <Maxim.Yanchenko@…>, 11 years ago

Cc: Maxim.Yanchenko@… added

Hi Ronald,

Is there any chance to have this patch properly applied? I understand your desire to refine the concepts and then introduce swap, if you apply the patch now the users would greatly benefit from this already now (after 2 years since the patch was proposed, actually, and 4 years since it was asked) and stop using ugly pseudo-solutions like shared_ptr<multi_array>. Then, when you refine the concepts, nothing will change in the existing production code (wrt swap) which is obviously good.

Please consider applying the patch even without your refinement of the operator=().

Thanks, Maxim

comment:9 by Ronald Garcia, 11 years ago

I am revisiting this issue and the broader issue of operator=() as I have time.

comment:10 by Rhys Ulerich <rhys.ulerich@…>, 11 years ago

Thanks for looking into it Ronald.

Just reading back over this chain of comments, and I noticed you said

Before I add swap to multi_array and multi_array_ref, I need to rename
the functionality currently associated with operator=() and remove
operator= from the concept.

but I don't read http://www.boost.org/doc/libs/1_48_0/libs/multi_array/doc/reference.html#MultiArray as saying anything at all about operator= within the MultiArray concept. No operator= specification. No Assignable requirement.

You also said

Then multi_array and multi_array_ref can
independently support a more traditional operator=() behavior, which
would be compatible with your swap.

but I'm not seeing how the proposed swap is incompatible with a naive swap built on multi_array::operator= or multi_array_ref::operator= in the case where their operator= precondition

std::equal(this->shape(),this->shape()+this->num_dimensions(), x.shape());

holds. The operator= postcondition will hold and operator== will report logical equivalence as expected after the swap. The proposed swap is a superset as it can handle situations when the precondition fails.

Maybe the only incompatibility I could see is whether or not the stride information in a given multi_array is preserved after a swap (naive swap: yes, proposed swap patch: no). But I do not expect (and generally would not want!) a swap that preserved stride information as it would always require a deep copy and defeat the purpose of a lightweight swap.

comment:11 by anonymous, 9 years ago

Now that we have move semantics, is it possible that we can solve the swap problem, w/out touching - at least, for now - the copy constructor? Thank you

comment:12 by anonymous, 5 years ago

Hi,

Is there any progress on this issue? As far as I can see multi_array still doesn't have a move constructor.

Thanks!

Note: See TracTickets for help on using tickets.