git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Justin Tobler <jltobler@gmail.com>,  git@vger.kernel.org,  ps@pks.im
Subject: Re: [PATCH 0/3] batch blob diff generation
Date: Wed, 18 Dec 2024 06:53:31 -0800	[thread overview]
Message-ID: <xmqqv7vhujo4.fsf@gitster.g> (raw)
In-Reply-To: <20241218113927.GB594795@coredump.intra.peff.net> (Jeff King's message of "Wed, 18 Dec 2024 06:39:27 -0500")

Jeff King <peff@peff.net> writes:

>> > I recall that the performance of attr.tree is not great for _some_
>> > commands (like pack-objects). So it's perhaps reasonable to use for
>> > single commands like "git diff" but not to set in your on-disk config.
>> > It's possible we'd want to warn people about that before advertising it
>> > more widely? I dunno.
>> 
>> Or we disable the unusably-inefficient feature before doing so.
>> Would attr.tree be much less efficient than GIT_ATTR_SOURCE?
>
> Whether it's unusably inefficient depends on what you throw at it. IIRC,
> the performance difference for pack-objects on git.git was fairly
> negligible. The problem in linux.git is that besides being big, it has a
> deep(er) directory structure. So collecting all of the attributes for a
> file like drivers/gpu/drm/foo/bar.h needs to open all of those
> intermediate trees.
>
> So I'd be inclined to leave it in place, in case somebody is actually
> happily using it.
>
> GIT_ATTR_SOURCE suffers all of the same problems; it's just that you'd
> presumably only use it with a few select commands (as far as I know,
> pack-objects is the worst case because it's looking up one attribute on
> every single blob in all of history).

Ah, OK.  So your "caution" was about the underlying mechanism to
allow attributes corrected from the specified tree, and not
specifically about using "attr.tree" to specify that tree?  That was
what got me confused.

If that is the case, I do not think the documentation patch that
started this exchange that adds attr.tree to where GIT_ATTR_SOURCE
and --attr-source are already mentioned makes anything worse.

Thanks.

  reply	other threads:[~2024-12-18 14:53 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-13  4:23 [PATCH 0/3] batch blob diff generation Justin Tobler
2024-12-13  4:23 ` [PATCH 1/3] builtin: introduce diff-blob command Justin Tobler
2024-12-13  4:23 ` [PATCH 2/3] builtin/diff-blob: add "--stdin" option Justin Tobler
2024-12-13  4:23 ` [PATCH 3/3] builtin/diff-blob: Add "-z" option Justin Tobler
2024-12-13  8:12 ` [PATCH 0/3] batch blob diff generation Jeff King
2024-12-13 10:17   ` Junio C Hamano
2024-12-13 10:38     ` Jeff King
2024-12-15  2:07       ` Junio C Hamano
2024-12-15  2:17         ` Junio C Hamano
2024-12-16 11:11           ` Jeff King
2024-12-16 16:29             ` Junio C Hamano
2024-12-18 11:39               ` Jeff King
2024-12-18 14:53                 ` Junio C Hamano [this message]
2024-12-20  9:09                   ` Jeff King
2024-12-20  9:10                     ` Jeff King
2024-12-13 16:41   ` Justin Tobler
2024-12-16 11:18     ` Jeff King
2024-12-13 22:34   ` Junio C Hamano
2024-12-15 23:24     ` Junio C Hamano
2024-12-16 11:30       ` Jeff King
2025-02-12  4:18 ` [PATCH v2 " Justin Tobler
2025-02-12  4:18   ` [PATCH v2 1/3] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-02-12  9:06     ` Karthik Nayak
2025-02-12 17:35       ` Justin Tobler
2025-02-12  9:23     ` Patrick Steinhardt
2025-02-12 17:24       ` Justin Tobler
2025-02-13  5:45         ` Patrick Steinhardt
2025-02-12  4:18   ` [PATCH v2 2/3] builtin: introduce diff-pairs command Justin Tobler
2025-02-12  9:23     ` Patrick Steinhardt
2025-02-12  9:51     ` Karthik Nayak
2025-02-25 23:38       ` Justin Tobler
2025-02-12 11:40     ` Jean-Noël Avila
2025-02-12 16:50     ` Junio C Hamano
2025-02-19 22:19       ` Justin Tobler
2025-02-19 23:19         ` Junio C Hamano
2025-02-19 23:47           ` Junio C Hamano
2025-02-20  0:32             ` Justin Tobler
2025-02-20 14:56               ` Justin Tobler
2025-02-20 16:14                 ` Junio C Hamano
2025-02-17 14:38     ` Phillip Wood
2025-02-19 20:51       ` Justin Tobler
2025-02-19 21:57         ` Junio C Hamano
2025-02-19 22:38           ` Justin Tobler
2025-02-26 14:47         ` Phillip Wood
2025-02-12  4:18   ` [PATCH v2 3/3] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler
2025-02-12  9:23     ` Patrick Steinhardt
2025-02-17 14:38     ` Phillip Wood
2025-02-19 23:09       ` Justin Tobler
2025-02-25 23:39   ` [PATCH v3 0/3] batch blob diff generation Justin Tobler
2025-02-25 23:39     ` [PATCH v3 1/3] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-02-26 18:04       ` Junio C Hamano
2025-02-25 23:39     ` [PATCH v3 2/3] builtin: introduce diff-pairs command Justin Tobler
2025-02-26 18:24       ` Junio C Hamano
2025-02-27 22:15         ` Justin Tobler
2025-02-27  9:35       ` Karthik Nayak
2025-02-27 22:36         ` Justin Tobler
2025-02-27 12:56       ` Patrick Steinhardt
2025-02-27 23:00         ` Justin Tobler
2025-02-25 23:39     ` [PATCH v3 3/3] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler
2025-02-26 14:58     ` [PATCH v3 0/3] batch blob diff generation phillip.wood123
2025-02-27 22:04       ` Justin Tobler
2025-02-28  0:26     ` [PATCH v4 0/4] " Justin Tobler
2025-02-28  0:26       ` [PATCH v4 1/4] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-02-28  0:26       ` [PATCH v4 2/4] diff: add option to skip resolving diff statuses Justin Tobler
2025-02-28  8:29         ` Patrick Steinhardt
2025-02-28 17:10           ` Justin Tobler
2025-02-28  0:26       ` [PATCH v4 3/4] builtin: introduce diff-pairs command Justin Tobler
2025-02-28  8:29         ` Patrick Steinhardt
2025-02-28 17:26           ` Justin Tobler
2025-02-28  0:26       ` [PATCH v4 4/4] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler
2025-02-28 21:33       ` [PATCH v5 0/4] batch blob diff generation Justin Tobler
2025-02-28 21:33         ` [PATCH v5 1/4] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-03-03 16:17           ` Junio C Hamano
2025-02-28 21:33         ` [PATCH v5 2/4] diff: add option to skip resolving diff statuses Justin Tobler
2025-03-03 16:19           ` Junio C Hamano
2025-02-28 21:33         ` [PATCH v5 3/4] builtin: introduce diff-pairs command Justin Tobler
2025-03-03 16:30           ` Junio C Hamano
2025-02-28 21:33         ` [PATCH v5 4/4] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqv7vhujo4.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /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.
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 NNTP newsgroup(s).