Re: [PATCH v2] unpack-trees: avoid duplicate ODB lookups during checkout

Am 10.04.2017 um 23:26 schrieb Jeff Hostetler:
 > On 4/8/2017 10:06 AM, René Scharfe wrote:
 >> Am 07.04.2017 um 17:53 schrieb git@jeffhostetler.com:
 >>> +            /* implicitly borrow buf[i-1] inside tree_desc[i] */
 >>> +            memcpy(&t[i], &t[i-1], sizeof(struct tree_desc));
 >>
 >> An assignment would be simpler:
 >>
 >>             t[i] = t[i - 1];
 >
 > True, but this might be a coin toss.  Maybe we should
 > see what the generated assembly looks like for each ??

Clang, GCC and ICC inline that memcpy call; the assembly output is the same for both variants: https://godbolt.org/g/1q0YwK. I guess you worry about compilers that are just bad at struct assignments (i.e. worse than calling memcpy)? Do you have examples (just curious)?

Assignments are easier on the eye of human readers in any case, and there is no way to silently get the size wrong.

 > I tried to hit the common cases.  This loop runs a lot
 > and I didn't want to put an O(n^2) thing in there to
 > look for any matching peer.  Most of the time we are
 > in a simple 2 or 3 way effort.  I didn't want to pay
 > for the looping/branching overhead for the obscure [4..8]
 > efforts.

Makes sense, and it's a nice heuristic. Perhaps it would be a good idea to document these choices in a comment?

René