From: Jeff Hostetler <git@jeffhostetler.com>
To: "René Scharfe" <l.s.r@web.de>, git@vger.kernel.org
Cc: gitster@pobox.com, peff@peff.net,
Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH v2] unpack-trees: avoid duplicate ODB lookups during checkout
Date: Tue, 11 Apr 2017 16:42:33 -0400 [thread overview]
Message-ID: <ad2b5a8c-f9d0-82cb-25b8-76c1922af7f5@jeffhostetler.com> (raw)
In-Reply-To: <001f55c6-a694-7dde-b14b-9d1dcc9c9a09@web.de>
On 4/10/2017 7:09 PM, René Scharfe wrote:
> 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)?
Nice website! Really!
Yes, my concern was that structure copies would do it
field by field rather than just a block copy. No, I
don't have any examples -- maybe just some very old
brain cells.... :-)
And I just checked VS2015 and the structure copy is a
few instructions shorter, but roughly the same.
>
> Assignments are easier on the eye of human readers in any case, and
> there is no way to silently get the size wrong.
agreed. thanks.
>
>> 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?
Good point. Thanks.
prev parent reply other threads:[~2017-04-11 20:42 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-07 15:53 [PATCH v2] unpack-trees: avoid duplicate ODB lookups during checkout git
2017-04-07 15:53 ` git
2017-04-08 14:06 ` René Scharfe
2017-04-10 20:55 ` Jeff King
2017-04-10 21:28 ` Jeff Hostetler
2017-04-10 21:26 ` Jeff Hostetler
2017-04-10 23:09 ` René Scharfe
2017-04-11 20:42 ` Jeff Hostetler [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ad2b5a8c-f9d0-82cb-25b8-76c1922af7f5@jeffhostetler.com \
--to=git@jeffhostetler.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jeffhost@microsoft.com \
--cc=l.s.r@web.de \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).