Code review comment for lp:~jobh/dolfin/fast-array

Revision history for this message
Joachim Haga (jobh) wrote :

This seems to have stalled without a consensus. Still, I think the improvements are too dramatic to just drop. Assembling a "v*f*dx" RHS on a 64^3 cube (twice):

6.95 s before
2.30 s after

With Garth's suggestion of reusing an Array: 2.54s, but not thread safe without some extra complexity (likely compiler-dependent).

I'd like to repeat my argument that my proposed change isn't any less safe than what's there already. The old Array used reference_to_no_delete_ptr in several constructors, thus the memory was managed by the caller -- exactly the same as it is here, if the Array is not the owner. Arrays were never shared [*], so the "shared" variant is exactly the same as the new "owned" variant.

Note [*]: It was used implicitly to transfer ownership in the TimeSeries return values. Since there's agreement than an extra copy does no harm there, I've changed those to return std::vector instead.

Then it can be deprecated or redesigned or whatever later, but in the meantime dolfin will be much faster :)

PS. The "interface version" thing seems to not work for all instant files, so instant-clean is still required. How should this be handled?

« Back to merge proposal