From: Junio C Hamano <gitster@pobox.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Jonathan Tan <jonathantanmy@google.com>, git@vger.kernel.org
Subject: Re: [PATCH v2 4/5] fetch: teach independent negotiation (no packfile)
Date: Thu, 06 May 2021 11:12:13 +0900 [thread overview]
Message-ID: <xmqqbl9o1ugy.fsf@gitster.g> (raw)
In-Reply-To: <9f650594-c8a6-4d89-2686-49de48aabb62@gmail.com> (Derrick Stolee's message of "Wed, 5 May 2021 12:42:09 -0400")
Derrick Stolee <stolee@gmail.com> writes:
>> Moving to lookup_object() made me look around, but at this point the
>> object in question (which comes from the negotiation_tips) has been
>> instantiated, so it is OK.
>>
>> Side note. The big difference between lookup and parse is what
>> happens when this process hasn't seen the object yet---the
>> former will just return NULL instead of instantiating the
>> in-core copy; for that reason, it is easier on the readers to
>> use parse_object() if we know we want an in-core object *and*
>> when we use it we want to see it parsed already).
>
> Please forgive my incorrect recommendation here. I was expecting
> lookup_object() to behave like lookup_commit(), which creates the
> object if it is not already in the cached set.
It is not wrong per-se to recommend or use lookup_object() in this
case, as long as who recommends or uses the function is certain that
the object should already exist in-core.
But it is just that the lookup_object() is often not what the caller
wants to use; it is a very rare use case for a caller to be able to
say "I am OK for you to return NULL to me for this object, even if
it is in our object store, when we do not yet have it in-core". The
only exception is the implementation of lookup_<type>() helpers,
where they handle "not loaded yet" answer from lookup_object() with
create_object() and always return usable in-core object to the
caller.
So the sole practical use case for lookup_object() outside the
lookup_<type>() implementation becomes "I am 100% sure that an
object with this object name has already been loaded in-core, so
please let me access it", which may be optimized for a wrong case
(because most of the API pass in-core objects, not object names,
around). I am not sure if this particular API strangeness can be
remedied, though.
Thanks.
next prev parent reply other threads:[~2021-05-06 2:12 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-09 1:09 [PATCH 0/6] Push negotiation Jonathan Tan
2021-04-09 1:09 ` [PATCH 1/6] fetch-pack: buffer object-format with other args Jonathan Tan
2021-04-09 4:49 ` Junio C Hamano
2021-04-09 16:24 ` Jonathan Tan
2021-04-09 1:09 ` [PATCH 2/6] fetch-pack: refactor process_acks() Jonathan Tan
2021-04-09 5:08 ` Junio C Hamano
2021-05-03 16:30 ` Derrick Stolee
2021-04-09 1:10 ` [PATCH 3/6] fetch-pack: refactor add_haves() Jonathan Tan
2021-04-09 5:20 ` Junio C Hamano
2021-04-09 1:10 ` [PATCH 4/6] fetch-pack: refactor command and capability write Jonathan Tan
2021-04-09 5:27 ` Junio C Hamano
2021-04-09 1:10 ` [PATCH 5/6] fetch: teach independent negotiation (no packfile) Jonathan Tan
2021-04-09 5:41 ` Junio C Hamano
2021-04-09 16:38 ` Jonathan Tan
2021-05-03 15:25 ` Derrick Stolee
2021-05-03 15:40 ` Derrick Stolee
2021-05-03 21:52 ` Jonathan Tan
2021-04-09 1:10 ` [PATCH 6/6] send-pack: support push negotiation Jonathan Tan
2021-05-03 15:35 ` Derrick Stolee
2021-05-03 22:02 ` Jonathan Tan
2021-05-04 17:26 ` Derrick Stolee
2021-04-30 5:42 ` [PATCH 0/6] Push negotiation Junio C Hamano
2021-04-30 17:33 ` Derrick Stolee
2021-05-04 21:15 ` [PATCH v2 0/5] " Jonathan Tan
2021-05-04 21:15 ` [PATCH v2 1/5] fetch-pack: refactor process_acks() Jonathan Tan
2021-05-04 21:15 ` [PATCH v2 2/5] fetch-pack: refactor add_haves() Jonathan Tan
2021-05-04 21:16 ` [PATCH v2 3/5] fetch-pack: refactor command and capability write Jonathan Tan
2021-05-04 21:16 ` [PATCH v2 4/5] fetch: teach independent negotiation (no packfile) Jonathan Tan
2021-05-05 1:53 ` Junio C Hamano
2021-05-05 16:42 ` Derrick Stolee
2021-05-06 2:12 ` Junio C Hamano [this message]
2021-05-05 16:44 ` Derrick Stolee
2021-05-04 21:16 ` [PATCH v2 5/5] send-pack: support push negotiation Jonathan Tan
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=xmqqbl9o1ugy.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=stolee@gmail.com \
/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).