From: Patrick Steinhardt <ps@pks.im>
To: Derrick Stolee <derrickstolee@github.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/5] upload-pack: look up "want" lines via commit-graph
Date: Tue, 1 Mar 2022 10:24:23 +0100 [thread overview]
Message-ID: <Yh3mR3/h/+nFyF6V@ncase> (raw)
In-Reply-To: <Yh3cprbfc/UQ01fo@ncase>
[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]
On Tue, Mar 01, 2022 at 09:43:19AM +0100, Patrick Steinhardt wrote:
> On Wed, Feb 23, 2022 at 09:13:53AM -0500, Derrick Stolee wrote:
> > On 2/23/2022 7:35 AM, Patrick Steinhardt wrote:
> >
> > > The following benchmark is executed in a repository with a huge number
> > > of references. It uses cached request from git-fetch(1) as input and
> > > contains about 876,000 "want" lines:
> > >
> > > Benchmark 1: git-upload-pack (HEAD~)
> > > Time (mean ± σ): 7.113 s ± 0.028 s [User: 6.900 s, System: 0.662 s]
> > > Range (min … max): 7.072 s … 7.168 s 10 runs
> > >
> > > Benchmark 2: git-upload-pack (HEAD)
> > > Time (mean ± σ): 6.622 s ± 0.061 s [User: 6.452 s, System: 0.650 s]
> > > Range (min … max): 6.535 s … 6.727 s 10 runs
> > >
> > > Summary
> > > 'git-upload-pack (HEAD)' ran
> > > 1.07 ± 0.01 times faster than 'git-upload-pack (HEAD~)'
> >
> > Nice!
> >
> > > - o = parse_object(the_repository, &oid);
> > > + commit = lookup_commit_in_graph(the_repository, &oid);
> > > + if (commit)
> > > + o = &commit->object;
> > > + else
> > > + o = parse_object(the_repository, &oid);
> > > +
> >
> > This is a neat trick. I see that we've also done this trick in
> > revision.c:get_reference(). Perhaps it is worth creating a helper,
> > maybe named parse_probably_commit()?
>
> That might be a good idea, thanks. I'll have a look at what the end
> result would look like.
>
> Patrick
I had a look at existing callsites which use `lookup_commit_in_graph()`,
but I found that it wasn't easily possible to convert them all to use a
new helper like you propose. Most of them have some custom logic like
skipping `parse_object()` if it's part of a promisor pack, so I really
only found two locations where such a new helper could be used without
also adding and supporting flags. I don't really think that's worth it
for now.
Patrick
> > > if (!o) {
> > > packet_writer_error(writer,
> > > "upload-pack: not our ref %s",
> > > @@ -1434,7 +1440,7 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,
> > > if (skip_prefix(line, "want-ref ", &refname_nons)) {
> > > struct object_id oid;
> > > struct string_list_item *item;
> > > - struct object *o;
> > > + struct object *o = NULL;
> > > struct strbuf refname = STRBUF_INIT;
> > >
> > > strbuf_addf(&refname, "%s%s", get_git_namespace(), refname_nons);
> > > @@ -1448,7 +1454,15 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,
> > > item = string_list_append(wanted_refs, refname_nons);
> > > item->util = oiddup(&oid);
> > >
> > > - o = parse_object_or_die(&oid, refname_nons);
> > > + if (!starts_with(refname_nons, "refs/tags/")) {
> > > + struct commit *commit = lookup_commit_in_graph(the_repository, &oid);
> > > + if (commit)
> > > + o = &commit->object;
> > > + }
> > > +
> > > + if (!o)
> > > + o = parse_object_or_die(&oid, refname_nons);
> > > +
> >
> > Even here, we _could_ use a parse_probably_commit() helper
> > inside the if (!starts_with(...)) block, even though we would
> > still need the if (!o) check later.
> >
> > Thanks,
> > -Stolee
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-03-01 9:24 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-23 12:35 [PATCH 0/5] fetch: more optimizations for mirror fetches Patrick Steinhardt
2022-02-23 12:35 ` [PATCH 1/5] upload-pack: look up "want" lines via commit-graph Patrick Steinhardt
2022-02-23 14:13 ` Derrick Stolee
2022-03-01 8:43 ` Patrick Steinhardt
2022-03-01 9:24 ` Patrick Steinhardt [this message]
2022-03-02 18:53 ` Derrick Stolee
2022-02-23 12:35 ` [PATCH 2/5] fetch: avoid lookup of commits when not appending to FETCH_HEAD Patrick Steinhardt
2022-02-23 14:18 ` Derrick Stolee
2022-03-01 8:44 ` Patrick Steinhardt
2022-02-23 12:35 ` [PATCH 3/5] refs: add ability for backends to special-case reading of symbolic refs Patrick Steinhardt
2022-02-23 12:35 ` [PATCH 4/5] remote: read symbolic refs via `refs_read_symbolic_ref()` Patrick Steinhardt
2022-02-23 12:35 ` [PATCH 5/5] refs/files-backend: optimize reading of symbolic refs Patrick Steinhardt
2022-03-01 9:33 ` [PATCH v2 0/5] fetch: more optimizations for mirror fetches Patrick Steinhardt
2022-03-01 9:33 ` [PATCH v2 1/5] upload-pack: look up "want" lines via commit-graph Patrick Steinhardt
2022-03-01 9:33 ` [PATCH v2 2/5] fetch: avoid lookup of commits when not appending to FETCH_HEAD Patrick Steinhardt
2022-03-01 9:33 ` [PATCH v2 3/5] refs: add ability for backends to special-case reading of symbolic refs Patrick Steinhardt
2022-03-01 9:33 ` [PATCH v2 4/5] remote: read symbolic refs via `refs_read_symbolic_ref()` Patrick Steinhardt
2022-03-01 9:33 ` [PATCH v2 5/5] refs/files-backend: optimize reading of symbolic refs Patrick Steinhardt
2022-03-01 22:02 ` [PATCH v2 0/5] fetch: more optimizations for mirror fetches Junio C Hamano
2022-03-02 18:54 ` Derrick Stolee
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=Yh3mR3/h/+nFyF6V@ncase \
--to=ps@pks.im \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
/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).