runtime: perf improvements for runtime.memmove for ppc64le/ppc64

[laboger]

There is a ppc64(le) specific implementation of memmove in runtime/memmove_ppc64x.s which works well in some cases but found some cases where further improvements can be made and plan to work on those. On inspection I also have some concerns about the alignment assumptions.

There are no alignment checks for the source and destination in this code. Is there an assumption when using the runtime.memmove function that these pointers will refer to data that is 8 byte aligned? If the length is >= 8 then it does ldu/stdu regardless of the alignment. I couldn't find any documentation stating that there is an expectation for the alignment of the source and destination with this function.

[randall77]

You do not get any alignment guarantee on the memmove inputs.
If ppc can't do unaligned loads and stores, it does need fixup code.
If there isn't yet, you should definitely add a test for this.

On Thu, Feb 25, 2016 at 8:51 AM, laboger [email protected] wrote:

There is a ppc64(le) specific implementation of memmove in
runtime/memmove_ppc64x.s which works well in some cases but found some
cases where further improvements can be made and plan to work on those. On
inspection I also have some concerns about the alignment assumptions.

There are no alignment checks for the source and destination in this code.
Is there an assumption when using the runtime.memmove function that these
pointers will refer to data that is 8 byte aligned? If the length is >= 8
then it does ldu/stdu regardless of the alignment. I couldn't find any
documentation stating that there is an expectation for the alignment of the
source and destination with this function.

—
Reply to this email directly or view it on GitHub
#14507.

[laboger]

The unaligned loads and stores will work but they are very inefficient. I handle that as part of my changes. Thanks.

[mwhudson]

It would be interesting to instrument memmove to see how it is called, both in terms of alignment and sizes. I think when copying anything containing pointers, you'll have at least pointer alignment -- maybe if this could be determined at the call site there would be a case for splitting memmove into one that assumes alignment and one that does not (and as we all know by now, if you are copying pointers you have to do it 8 bytes at a time).

[randall77]

It might be informative to break memmove calls down by type. There are:

1. Calls for runtime generics (maps, channels, interfaces, etc.)
2. Calls for append
3. Calls for string <-> []byte
4. Calls for compiler-generated copying of big user types

#1,#2, and #4 are almost always aligned. Although very likely, I think we could only guarantee it at #2 and #4 callsites. Alignment in case #3 is less likely.
For #2 and #4, the copies are typically large so a test and branch for alignment wouldn't add much relative cost.

[mwhudson]

Isn't it more fair to say that the alignment of the arguments to memmove match the alignment of the type being copied? When appending two []byte's, there's no guarantee of alignment. And so the question becomes whether the type's alignment is known at the callsite to memmove – I think for the cases you list, the calls are all via typedmemmove, which does have that information, so maybe it would be practical to have a separate memmove_aligned function.

The size distribution is interesting too, there's always this temptation (for me, anyway) to focus on the asymptotic times and making the larger copies as fast as possible, but I don't think that's the way to go for most actual Go programs (but I might be wrong; I'd love for someone to do something rigorous here)

[randall77]

typedmemmove is itself generic. So I guess one test/branch in typedmemmove would ensure that both the source and destination are aligned and could call a specialized memmove routine. We can do the same thing in memmove itself with an or/test/branch. Doesn't seem like a big win.

I agree with your size feelings. It's more important for Go to get the 1 and 2 word memmoves fast. Of course it depends a lot on the benchmark.

[mwhudson]

I was thinking typedmemmove could check t.align but I guess that doesn't
buy much over checking the alignment in memmove indeed.

On 1 March 2016 at 05:18, Keith Randall [email protected] wrote:

typedmemmove is itself generic. So I guess one test/branch in typedmemmove
would ensure that both the source and destination are aligned and could
call a specialized memmove routine. We can do the same thing in memmove
itself with an or/test/branch. Doesn't seem like a big win.

I agree with your size feelings. It's more important for Go to get the 1
and 2 word memmoves fast. Of course it depends a lot on the benchmark.

—
Reply to this email directly or view it on GitHub
#14507 (comment).

[laboger]

I'm sorry but I was wrong in my previous comment about the performance of unaligned loads and stores on power8. On earlier Power instruction sets unaligned loads and stores were very inefficient but I have since found out that on power8 unaligned loads and stores perform well. So for now I think the main areas of performance improvement for the memmove are when 1) figuring out if the src and dst overlap (I don't think the current test is always correct and may decide to use backward moves when not necessary) and 2) when moving large amounts of data. In the second case, unrolling the loop and using dcache hints will help. I'm doing more experiments and getting input from other power8 developers.

[laboger]

https://go-review.googlesource.com/#/c/21990/

[gopherbot]

CL https://golang.org/cl/21990 mentions this issue.