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

I am in favor of applying the patch.

On Tue, Mar 6, 2012 at 12:20 PM, Joachim Haga <email address hidden> 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?

Not sure what is not working, but it should be enough to change the
DOLFIN_VERSION_MICRO, as this is eventualy included in the md5sum
which determines if a modules should be recompiled or not.

Johan

> --
> https://code.launchpad.net/~jobh/dolfin/fast-array/+merge/94467
> Your team DOLFIN Core Team is requested to review the proposed merge of lp:~jobh/dolfin/fast-array into lp:dolfin.