From: Junio C Hamano <gitster@pobox.com>
To: Eric Wong <e@80x24.org>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v2 05/10] cat-file: use delta_base_cache entries directly
Date: Mon, 26 Aug 2024 16:05:17 -0700 [thread overview]
Message-ID: <xmqqcyluga1e.fsf@gitster.g> (raw)
In-Reply-To: <xmqqjzg3geea.fsf@gitster.g> (Junio C. Hamano's message of "Mon, 26 Aug 2024 14:31:09 -0700")
Junio C Hamano <gitster@pobox.com> writes:
>> + /*
>> + * Set if caller is able to use OI_DBCACHED entries without copying.
>> + * This only applies to OI_DBCACHED entries at the moment,
>> + * not OI_CACHED or any other type of entry.
>> + */
>> + unsigned direct_cache:1;
> ...
> You seem to set this bit in batch_objects(), so it does sound like
> that the bit is expected to be set by the caller to tell the API
> something. If that is the case, then (1) move it to "request" part
> of the object_info structure, and (2) define what it means to be
> "able to use ... without copying". Mechanically, it may mean
> "contentp directly points into the delta base cache", but what
> implication does it have to callers? If the caller obtains such a
> pointer in .contentp, what is required for its access pattern to
> make accessing the pointer safe? The caller cannot use the pointed
> memory after it accesses another object? What is the definition of
> "access" in the context of this discussion? Does "checking for
> existence of an object" count as an access?
Another thing that makes me worry about this approach (as opposed to
a much simpler to reason about alternative like "transfer ownership
to the caller") is that it is very hard to guarantee that other
object access that is not under caller's control will never happen,
and it is even harder to make sure that the code will keep giving
such a guarantee.
In other words, the arrangement smells a bit too brittle.
For example, would it be possible for a lazy fetching of (unrelated)
objects triggers after the caller asks about an object and borrows a
pointer into delta base cache, and would it scramble the entries of
delta base cache, silently invalidating the borrowed piece of
memory? Would it be possible for the textconv and other filtering
mechanism driven by the attribute system trigger access to
configured attribute blob, which has to be lazily fetched from other
place?
Thanks.
next prev parent reply other threads:[~2024-08-26 23:05 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-15 0:35 [PATCH v1 00/10] cat-file speedups Eric Wong
2024-07-15 0:35 ` [PATCH v1 01/10] packfile: move sizep computation Eric Wong
2024-07-24 8:35 ` Patrick Steinhardt
2024-07-15 0:35 ` [PATCH v1 02/10] packfile: allow content-limit for cat-file Eric Wong
2024-07-24 8:35 ` Patrick Steinhardt
2024-07-26 7:30 ` Eric Wong
2024-07-15 0:35 ` [PATCH v1 03/10] packfile: fix off-by-one in content_limit comparison Eric Wong
2024-07-24 8:35 ` Patrick Steinhardt
2024-07-26 7:43 ` Eric Wong
2024-07-15 0:35 ` [PATCH v1 04/10] packfile: inline cache_or_unpack_entry Eric Wong
2024-07-15 0:35 ` [PATCH v1 05/10] cat-file: use delta_base_cache entries directly Eric Wong
2024-07-24 8:35 ` Patrick Steinhardt
2024-07-26 7:42 ` Eric Wong
2024-08-18 17:36 ` assert vs BUG [was: [PATCH v1 05/10] cat-file: use delta_base_cache entries directly] Eric Wong
2024-08-19 15:50 ` Junio C Hamano
2024-07-15 0:35 ` [PATCH v1 06/10] packfile: packed_object_info avoids packed_to_object_type Eric Wong
2024-07-24 8:36 ` Patrick Steinhardt
2024-07-26 8:01 ` Eric Wong
2024-07-15 0:35 ` [PATCH v1 07/10] object_info: content_limit only applies to blobs Eric Wong
2024-07-15 0:35 ` [PATCH v1 08/10] cat-file: batch-command uses content_limit Eric Wong
2024-07-15 0:35 ` [PATCH v1 09/10] cat-file: batch_write: use size_t for length Eric Wong
2024-07-15 0:35 ` [PATCH v1 10/10] cat-file: use writev(2) if available Eric Wong
2024-07-24 8:35 ` [PATCH v1 00/10] cat-file speedups Patrick Steinhardt
2024-08-23 22:46 ` [PATCH v2 " Eric Wong
2024-08-23 22:46 ` [PATCH v2 01/10] packfile: move sizep computation Eric Wong
2024-09-17 10:06 ` Taylor Blau
2024-08-23 22:46 ` [PATCH v2 02/10] packfile: allow content-limit for cat-file Eric Wong
2024-08-26 17:10 ` Junio C Hamano
2024-08-27 20:23 ` Eric Wong
2024-09-17 10:10 ` Taylor Blau
2024-09-17 21:15 ` Junio C Hamano
2024-08-23 22:46 ` [PATCH v2 03/10] packfile: fix off-by-one in content_limit comparison Eric Wong
2024-08-26 16:55 ` Junio C Hamano
2024-09-17 10:11 ` Taylor Blau
2024-08-23 22:46 ` [PATCH v2 04/10] packfile: inline cache_or_unpack_entry Eric Wong
2024-08-26 17:09 ` Junio C Hamano
2024-10-06 17:40 ` Eric Wong
2024-08-23 22:46 ` [PATCH v2 05/10] cat-file: use delta_base_cache entries directly Eric Wong
2024-08-26 21:31 ` Junio C Hamano
2024-08-26 23:05 ` Junio C Hamano [this message]
2024-08-23 22:46 ` [PATCH v2 06/10] packfile: packed_object_info avoids packed_to_object_type Eric Wong
2024-08-26 21:50 ` Junio C Hamano
2024-08-23 22:46 ` [PATCH v2 07/10] object_info: content_limit only applies to blobs Eric Wong
2024-08-26 22:02 ` Junio C Hamano
2024-08-23 22:46 ` [PATCH v2 08/10] cat-file: batch-command uses content_limit Eric Wong
2024-08-26 22:13 ` Junio C Hamano
2024-08-23 22:46 ` [PATCH v2 09/10] cat-file: batch_write: use size_t for length Eric Wong
2024-08-27 5:06 ` Junio C Hamano
2024-08-23 22:46 ` [PATCH v2 10/10] cat-file: use writev(2) if available Eric Wong
2024-08-27 5:41 ` Junio C Hamano
2024-08-27 15:43 ` Junio C Hamano
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=xmqqcyluga1e.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=e@80x24.org \
--cc=git@vger.kernel.org \
--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).