From: Derrick Stolee <derrickstolee@github.com>
To: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Subject: Re: [PATCH 1/5] upload-pack: look up "want" lines via commit-graph
Date: Wed, 23 Feb 2022 09:13:53 -0500 [thread overview]
Message-ID: <ddb91a1d-6ddf-ba25-f1af-ba3f4c18726e@github.com> (raw)
In-Reply-To: <ca5e136cca495c7d927e99f5ae8a672d93823eea.1645619224.git.ps@pks.im>
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()?
> 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
next prev parent reply other threads:[~2022-02-23 14:14 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 [this message]
2022-03-01 8:43 ` Patrick Steinhardt
2022-03-01 9:24 ` Patrick Steinhardt
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=ddb91a1d-6ddf-ba25-f1af-ba3f4c18726e@github.com \
--to=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--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.
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).