add fftw_copy_plan

[stevengj]

Closes #106.

Since plan execution is read-only, it seemed sufficient to implement this simply by adding an internal reference count to fftw_plan, so that fftw_plan just increments the counter and fftw_destroy plan decrements the counter, only de-allocating the plan when the reference count drops to zero.

@matteo-frigo, what do you think?

[matteo-frigo]

My first reaction is to say that this is a bad idea.

If the use case is C++ wrappers, C++ already has standard facilities for wrapping plain old pointers into reference-counted objects, with a menu of mutually exclusive semantics (shared vs unique vs move). I don't see what problem is solved by reimplementing this functionality at a lower level.

Is there some case where std::make_shared doesn't do what you need?

Or perhaps you have some other language in mind?

[stevengj]

I was actually thinking of Julia and Python, if they want to implement a deepcopy method. Of course, in those languages one can (and does) already wrap an fftw_plan in a garbage-collected object, which often contains some other information as well (e.g. about the size and type of array that the plan is applicable to, since we don't have a way to extract this from the fftw_plan), especially in Python where you can add attributes to an object dynamically. If for some reason the user requests a deepcopy of this object you are supposed to return a new high-level object, but since you can't copy the fftw_plan you'd need to either forbid deepcopy or wrap the plan pointer in another layer of garbage collection.

It's not a huge deal, just a slight annoyance that there is no way to copy a plan in the FFTW API, and easy for us to provide.

And of course it might be useful in C and low-level languages as well, which don't have built-in reference counting or garbage collection, if you want to pass plan pointers around without having to keep track of who owns the pointer.

[stevengj]

I'm not sure how you can use std::make_shared with an fftw_plan, since I thought std::make_shared does not support custom deleters? I guess you mean using something like std::shared_ptr<fftw_plan_s> plan(fftw_plan_dft_1d(...), fftw_destroy_plan);

[matteo-frigo]

I don't know, it still smells like a bad idea to me, but if you think this is needed, go ahead and push it.

My main objection is that this change doesn't solve any real problems, since I am afraid that every real use case will end up wrapping a plan into some other high-level object, at which point you may as well use native facilities to resource-manage the object. You allude to this problem yourself when you say that the object must contain some other information anyway, e.g. array size.

Your comment on make_shared is valid. I had in mind the case where the plan is wrapped in a C++ object, because no self-respecting C++ programmer would be caught dead using a raw C pointer.

But I don't feel strongly about this issue. If you push it, maybe add a test somewhere.

[stevengj]

There is already a test (for every plan the test program creates)

[stevengj]

We should probably set up GitHub CI at some point…

[stevengj]

I'm going to go ahead and merge.

[correaa]

I also badly need to make copies, but not in this form.

I looked at the implementation; this is just a reference counter "copy"; this defeats the purpose of having a copy. Then I have to agree that this reference counting, which is not very useful, could be implemented at a higher level, and it is a bad idea to have it at this low level.

The idea of having independent copies of the plans is not about multiple objects that share a reference; but it is about multiple equivalent plans that can be used simultaneously from different threads without internal locks. (So, allocated buffers (or any internal state) are not shared.) Of course, even if I could store the parameters of plan creation, I wouldn't want to pay for the estimation/measurement: I want to make a cheaper copy (which probably also entails allocating independent internal buffers).

In my reading, this PR is not in the spirit of the original request: https://github.com/FFTW/fftw3/issues/106.

PS1: I am also implementing yet another FFTW C++ wrapper; in my case, I want to make the wrapper thread-compatible (not necessarily thread-safe, but thread-friendly).

PS2: I always assumed this would be eventually implemented with a fftw_sscan_plan, analogous to fftw_sprint_plan.

[matteo-frigo]

@correaa fftw plans are already "thread safe" in the sense that the plan is read-only when executing the plan, and thus you can execute the same plan from multiple threads. Plans do not store buffers, but they do store immutable arrays of trigonometric constants and other read-only information. So I don't think that the problem you mention needs to be solved.

[correaa]

@correaa fftw plans are already "thread-safe" in the sense that the plan is read-only when executing the plan, and thus you can execute the same plan from multiple threads. Plans do not store buffers, but they do store immutable arrays of trigonometric constants and other read-only information. So I don't think that the problem you mention needs to be solved.

OK, I have been awfully mistaken for a while then.

I was convinced that plan execution (over disjoint targets) was not thread-safe. In my mind, it made sense because I assumed that the plan creation would allocate buffers to perform the ffts, so executions themselves wouldn't need to do it at that point.

I was so convinced about this that I misread the documentation: http://www.fftw.org/fftw3_doc/Thread-safety.html, which was silly on my side. I confused the comment about planner (plan creation) with the plan execution itself.

In that case, I wonder if there are buffers behind the scenes in some global state of the library. And if so, how they are synchronized? (I can be mistaken again but it looks like since the planning is not thread safe then there is a lot of global state behind the scenes.)

Anyhow, given that:

1) "Real" or ref counted copies are not that necessary then.

2) "Real" copies would still be good to have if, in the future, the plans would need to have (benefit from having) mutable state or buffers (during execution). (This could help the library to have less global state, but I understand if this is not a priority)

3) Reference counting still seems to me to be out of place here.

4) More importantly, from a C++ perspective (if you are interested at all), I can now finally declare execute as a const method with this knowledge.

5) In effect, because the of all this new knowledge (to me) I can safely implement this same thing with shared_ptr without waiting for this new plan_copy function to appear in my FFTW distribution.

Thank you for the rapid answer.

[matteo-frigo]

I wonder if there are buffers behind the scenes in some global state of the library. And if so, how they are synchronized?

There are no buffers in plans. When a plan needs a buffer it calls malloc(), or alloca() when the buffer is really small.

I think I agree with all your points 1-5 (see my Mar 2 comment), but other people feel otherwise.

[correaa]

I wonder if there are buffers behind the scenes in some global state of the library. And if so, how they are synchronized?

There are no buffers in plans. When a plan needs a buffer it calls malloc(), or alloca() when the buffer is really small.

I am surprised about that also. (I am surprised often).

Unless I am missing something, once the library decides what plan parameters to use, the amount of memory needed for execution seems to be determined. That is why I also assumed (wrongly again) that 1) allocation of the buffers happened on plan creation and 2) the plan would own the buffers.

I think I agree with all your points 1-5 (see my Mar 2 comment), but other people feel otherwise.

Yes, this is IMO why it is so hard to write a good C++ wrapper for FFTW, because there are very subtle semantic implications that are hard to capture with simple classes.

The other problem with naive reference counting is that it is not thread-safe itself. In std::shared_ptr, the counting is thread-safe; that is what makes it useful (beyond simply simulating garbage collection)

Anyway, food for thought for FFTW4 :)

Cheers!