[gmx-developers] std::vector<Rvec> slowness
Mark Abraham
mark.j.abraham at gmail.com
Wed Mar 23 13:43:28 CET 2016
Hi,
RVec was intended as a convenience feature for people writing high-level,
rather than low-level code. I would indeed not advocate using it in
performance-sensitive mdrun code without checking for such performance
effects.
I think the real issue is closer to that the use of *rvec* generally
confuses compilers because annotations about pointers do indeed get lost,
and perhaps [3] in real [3][] is not as close to being part of the type in
the compiler implementation as we might expect. (For a long time, we've
used very little rvec close to the short-ranged kernels. The first thing we
do with the rvec * args to the SIMD bondeds is to cast them to real *. I
think this is just as hard to police as Berk's concern about not using
std::vector<RVec>::operator[] naively - the problem isn't the "fault" of
std::vector or its use.)
The use of rvec * engenders some transposing that might be avoided if we
had SoA rather than AoS layout in mdrun. A single rvec is likely to have
all three coordinates on the same cache line, but if one is anyway often
using nearby atoms the caching behaviour of three separate real * arrays
should be rather similar. One still tends to have gather/scatter overhead,
however.
Mark
On Wed, Mar 23, 2016 at 11:24 AM Berk Hess <hess at kth.se> wrote:
> No.
> using the RVec * pointer obtained through .data() already fixes the issue.
> We don't even need as_rvec_array().
>
> The real issue is that it's near impossible to enforce this and detect
> vector access of RVecs if we start using std::vector everywhere through the
> code.
>
> Cheers,
>
> Berk
>
>
> On 2016-03-23 11:19, Mark Abraham wrote:
>
> Hi,
>
> If so, then we might want a helper function like as_rvec_array, but
> as_real_array instead.
>
> Mark
>
> On Wed, Mar 23, 2016 at 10:51 AM Berk Hess <hess at kth.se> wrote:
>
>> On 2016-03-23 10:50, Erik Lindahl wrote:
>>
>> Hi,
>>
>> I haven’t followed the discussion in detail, but a long time ago I
>> remember having simliar issues in the kernels when using a list[] of rvec[]
>> (in plain-old-c, no c++) instead of extracting the pointer and handling the
>> multiply-by-3 manually. Could it be something similar here, e.g. that the
>> compiler things it does not know enough about RVec rather than something
>> going from with the outer list?
>>
>> That would be my guess. The index used in the same loop comes from a
>> vector as well and doesn't seem to affect performance.
>>
>>
>> Berk
>>
>>
>> Cheers,
>>
>> Erik
>>
>> On 23 Mar 2016, at 10:44, Berk Hess <hess at kth.se> wrote:
>>
>> On 2016-03-23 10:42, Mark Abraham wrote:
>>
>> Hi,
>>
>> On Wed, Mar 23, 2016 at 9:44 AM Berk Hess <hess at kth.se> wrote:
>>
>>> Hi,
>>>
>>> Luckily Szilard does thorough testing and noticed a performance
>>> degradation in change set 25 of https://gerrit.gromacs.org/#/c/5232/ The
>>> only signifcant change with respect to previous sets is replacing C
>>> pointers by std::vector. I traced the performance difference back to a
>>> single loop, which must have become several factors slower to explain
>>> the time difference. I get the performance back when replacing the
>>> vector by a pointer extracted with .data(), see below. I looked at the
>>> assembly code from gcc 5.3.1 and the vector case generated 200 extra
>>> instructions, which makes it difficult to see what the actual difference
>>> is. The pointer case uses a lot of vmovss and vaddss, which the vector
>>> one does much less, but this is only serial SIMD instruction. I thought
>>> that [] in vector might does bounds checking,
>>
>>
>> Definitely not in release builds.
>>
>>
>>> but it seems it does not.
>>> Can anyone explain why the vector case can be so slow?
>>
>> If this is a general issue (with RVec or more?), we need to always extra
>>> a pointer with .data() for use in all inner-loops. This is pretty
>>> annoying and difficult to enforce.
>>>
>>> Cheers,
>>>
>>> Berk
>>>
>>> const std::vector<RVec> f_foreign =
>>> idt_foreign->force
>>>
>>
>> This does a copy of the vector, and doesn't seem to be in any version of
>> this patch in gerrit. Is this what you meant to write?
>>
>> I tried this. But my original "vectorized" patch set took a pointer from
>> idt_foreign and did not copy the vector, that gives the same, slow,
>> performance.
>>
>> Berk
>>
>>
>> Mark
>>
>> or
>>> const RVec *f_foreign =
>>> idt_foreign->force.data();
>>>
>>> int natom = atomList->atom.size();
>>> for (int i = 0; i < natom; i++)
>>> {
>>> int ind = atomList->atom[i];
>>> rvec_inc(f[ind], f_foreign[ind]);
>>> }
>>> --
>>> Gromacs Developers mailing list
>>>
>>> * Please search the archive at
>>> http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List before
>>> posting!
>>>
>>> * Can't post? Read http://www.gromacs.org/Support/Mailing_Lists
>>>
>>> * For (un)subscribe requests visit
>>> https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers
>>> or send a mail to gmx-developers-request at gromacs.org.
>>>
>>
>>
>>
>> --
>> Gromacs Developers mailing list
>>
>> * Please search the archive at
>> http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List before
>> posting!
>>
>> * Can't post? Read http://www.gromacs.org/Support/Mailing_Lists
>>
>> * For (un)subscribe requests visit
>> https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers
>> or send a mail to gmx-developers-request at gromacs.org.
>>
>>
>>
>>
>>
>> --
>> Gromacs Developers mailing list
>>
>> * Please search the archive at
>> http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List before
>> posting!
>>
>> * Can't post? Read http://www.gromacs.org/Support/Mailing_Lists
>>
>> * For (un)subscribe requests visit
>> https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers
>> or send a mail to gmx-developers-request at gromacs.org.
>
>
>
>
> --
> Gromacs Developers mailing list
>
> * Please search the archive at
> http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List before
> posting!
>
> * Can't post? Read http://www.gromacs.org/Support/Mailing_Lists
>
> * For (un)subscribe requests visit
> https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers
> or send a mail to gmx-developers-request at gromacs.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20160323/fac59b21/attachment-0001.html>
More information about the gromacs.org_gmx-developers
mailing list