<div dir="ltr">Hi,<br><br><div class="gmail_quote"><div dir="ltr">On Wed, Mar 23, 2016 at 1:56 PM Berk Hess <<a href="mailto:hess@kth.se">hess@kth.se</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div>Hi,<br>
<br>
It's impractical and bad for performance to go from AoS to SoA.<br></div></div></blockquote><div><br></div><div>Sure, but that's what we do every MD step on typical CPUs. We do DD communication on XYZXYZXYZXYZ and then transpose them to XXXXYYYYZZZZ to use in short-ranged kernels. If we did DD communication on XXXXX....YYYYY..... ZZZZZZ.... then we still have gather-scatter overhead, but we don't have transposition in the mix.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><div>
The question is now if we can fix this issue or if we should
always try to use .data() in inner-loops</div></div></blockquote><div><br></div><div>I think you're missing my point... we've always done a cast from rvec * to real * to pass to inner loops. :-) Using data() is a similar idea - hence my suggestion of as_real_array(theVectorOfRVecs).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><div> or if we should
completely avoid std::vector<RVec> in performance sensitive
code and use good old C pointers with nasty manual allocation.<br></div></div></blockquote><div><br></div><div>Allocation is irrelevant to how one uses it. One must have a real * to be sure the compiler isn't confused. To get that from a vector, we use data(). Whether the allocation of the memory is managed by std::vector or snew is immaterial.</div><div><br></div><div>Mark</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><div>
Cheers,<br>
<br>
Berk</div></div><div text="#000000" bgcolor="#FFFFFF"><div><br>
<br>
On 2016-03-23 13:43, Mark Abraham wrote:<br>
</div></div><div text="#000000" bgcolor="#FFFFFF">
<blockquote type="cite">
<div dir="ltr">Hi,
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.)</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Mark</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Mar 23, 2016 at 11:24 AM Berk Hess <<a href="mailto:hess@kth.se" target="_blank"></a><a href="mailto:hess@kth.se" target="_blank">hess@kth.se</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div>No.<br>
using the RVec * pointer obtained through .data() already
fixes the issue. We don't even need as_rvec_array().<br>
<br>
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.<br>
<br>
Cheers,<br>
<br>
Berk</div>
</div>
<div text="#000000" bgcolor="#FFFFFF">
<div><br>
<br>
On 2016-03-23 11:19, Mark Abraham wrote:<br>
</div>
</div>
<div text="#000000" bgcolor="#FFFFFF">
<blockquote type="cite">
<div dir="ltr">Hi,
<div><br>
</div>
<div>If so, then we might want a helper function like
as_rvec_array, but as_real_array instead.</div>
<div><br>
</div>
<div>Mark</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Mar 23, 2016 at 10:51 AM Berk
Hess <<a href="mailto:hess@kth.se" target="_blank">hess@kth.se</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div>On 2016-03-23 10:50, Erik Lindahl wrote:<br>
</div>
<blockquote type="cite"> Hi,
<div><br>
</div>
<div>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?</div>
</blockquote>
</div>
<div text="#000000" bgcolor="#FFFFFF"> 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.</div>
<div text="#000000" bgcolor="#FFFFFF"><br>
<br>
Berk</div>
<div text="#000000" bgcolor="#FFFFFF"><br>
<blockquote type="cite">
<div><br>
</div>
<div>Cheers,</div>
<div><br>
</div>
<div>Erik</div>
<div><br>
</div>
<div>
<div>
<blockquote type="cite">
<div>On 23 Mar 2016, at 10:44, Berk Hess
<<a href="mailto:hess@kth.se" target="_blank">hess@kth.se</a>>
wrote:</div>
<br>
<div>
<div text="#000000" bgcolor="#FFFFFF">
<div>On 2016-03-23 10:42, Mark Abraham
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi,<br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Mar 23,
2016 at 9:44 AM Berk Hess <<a href="mailto:hess@kth.se" target="_blank"></a><a href="mailto:hess@kth.se" target="_blank">hess@kth.se</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Luckily Szilard does thorough
testing and noticed a
performance<br>
degradation in change set 25 of
<a href="https://gerrit.gromacs.org/#/c/5232/" target="_blank">https://gerrit.gromacs.org/#/c/5232/</a>
The<br>
only signifcant change with
respect to previous sets is
replacing C<br>
pointers by std::vector. I
traced the performance
difference back to a<br>
single loop, which must have
become several factors slower to
explain<br>
the time difference. I get the
performance back when replacing
the<br>
vector by a pointer extracted
with .data(), see below. I
looked at the<br>
assembly code from gcc 5.3.1 and
the vector case generated 200
extra<br>
instructions, which makes it
difficult to see what the actual
difference<br>
is. The pointer case uses a lot
of vmovss and vaddss, which the
vector<br>
one does much less, but this is
only serial SIMD instruction. I
thought<br>
that [] in vector might does
bounds checking,</blockquote>
<div><br>
</div>
<div>Definitely not in release
builds.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> but it
seems it does not.<br>
Can anyone explain why the
vector case can be so slow?</blockquote>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> If this
is a general issue (with RVec or
more?), we need to always extra<br>
a pointer with .data() for use
in all inner-loops. This is
pretty<br>
annoying and difficult to
enforce.<br>
<br>
Cheers,<br>
<br>
Berk<br>
<br>
const
std::vector<RVec>
f_foreign =<br>
idt_foreign->force<br>
</blockquote>
<div><br>
</div>
<div>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?</div>
</div>
</div>
</blockquote>
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.<br>
<br>
Berk<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>Mark</div>
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> or<br>
const
RVec *f_foreign
=<br>
idt_foreign->force.data();<br>
<br>
int
natom =
atomList->atom.size();<br>
for
(int i = 0; i < natom; i++)<br>
{<br>
int
ind = atomList->atom[i];<br>
rvec_inc(f[ind],
f_foreign[ind]);<br>
}<br>
--<br>
Gromacs Developers mailing list<br>
<br>
* Please search the archive at <a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" target="_blank"></a><a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" target="_blank">http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List</a>
before posting!<br>
<br>
* Can't post? Read <a href="http://www.gromacs.org/Support/Mailing_Lists" target="_blank"></a><a href="http://www.gromacs.org/Support/Mailing_Lists" target="_blank">http://www.gromacs.org/Support/Mailing_Lists</a><br>
<br>
* For (un)subscribe requests
visit<br>
<a href="https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers" rel="noreferrer" target="_blank">https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers</a>
or send a mail to <a href="mailto:gmx-developers-request@gromacs.org" target="_blank"></a><a href="mailto:gmx-developers-request@gromacs.org" target="_blank">gmx-developers-request@gromacs.org</a>.<br>
</blockquote>
</div>
</div>
<br>
<fieldset></fieldset>
<br>
</blockquote>
<br>
</div>
-- <br>
Gromacs Developers mailing list<br>
<br>
* Please search the archive at <a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" target="_blank"></a><a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" target="_blank">http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List</a>
before posting!<br>
<br>
* Can't post? Read <a href="http://www.gromacs.org/Support/Mailing_Lists" target="_blank"></a><a href="http://www.gromacs.org/Support/Mailing_Lists" target="_blank">http://www.gromacs.org/Support/Mailing_Lists</a><br>
<br>
* For (un)subscribe requests visit<br>
<a href="https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers" target="_blank">https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers</a>
or send a mail to <a href="mailto:gmx-developers-request@gromacs.org" target="_blank"></a><a href="mailto:gmx-developers-request@gromacs.org" target="_blank">gmx-developers-request@gromacs.org</a>.</div>
</blockquote>
</div>
<br>
</div>
<br>
<fieldset></fieldset>
<br>
</blockquote>
<br>
</div>
--<br>
Gromacs Developers mailing list<br>
<br>
* Please search the archive at <a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" rel="noreferrer" target="_blank"></a><a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" target="_blank">http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List</a>
before posting!<br>
<br>
* Can't post? Read <a href="http://www.gromacs.org/Support/Mailing_Lists" rel="noreferrer" target="_blank">http://www.gromacs.org/Support/Mailing_Lists</a><br>
<br>
* For (un)subscribe requests visit<br>
<a href="https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers" rel="noreferrer" target="_blank">https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers</a>
or send a mail to <a href="mailto:gmx-developers-request@gromacs.org" target="_blank">gmx-developers-request@gromacs.org</a>.</blockquote>
</div>
<br>
<fieldset></fieldset>
<br>
</blockquote>
<br>
</div>
--<br>
Gromacs Developers mailing list<br>
<br>
* Please search the archive at <a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" rel="noreferrer" target="_blank">http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List</a>
before posting!<br>
<br>
* Can't post? Read <a href="http://www.gromacs.org/Support/Mailing_Lists" rel="noreferrer" target="_blank">http://www.gromacs.org/Support/Mailing_Lists</a><br>
<br>
* For (un)subscribe requests visit<br>
<a href="https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers" rel="noreferrer" target="_blank">https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers</a>
or send a mail to <a href="mailto:gmx-developers-request@gromacs.org" target="_blank">gmx-developers-request@gromacs.org</a>.</blockquote>
</div>
<br>
<fieldset></fieldset>
<br>
</blockquote>
<br>
</div>
--<br>
Gromacs Developers mailing list<br>
<br>
* Please search the archive at <a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" rel="noreferrer" target="_blank">http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List</a> before posting!<br>
<br>
* Can't post? Read <a href="http://www.gromacs.org/Support/Mailing_Lists" rel="noreferrer" target="_blank">http://www.gromacs.org/Support/Mailing_Lists</a><br>
<br>
* For (un)subscribe requests visit<br>
<a href="https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers" rel="noreferrer" target="_blank">https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers</a> or send a mail to <a href="mailto:gmx-developers-request@gromacs.org" target="_blank">gmx-developers-request@gromacs.org</a>.</blockquote></div></div>