Skip to content

keep track of unresolved value of symbolic-ref in ref iterators

John Cai via GitGitGadget git 4 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v4 Latest3 patches 2 repliesIn review
  2. v33 patches 2 repliesSuperseded
  3. v23 patches 8 repliesSuperseded
Earlier versions · 1
VersionPostedPatchesRepliesStatus
v14 patches15 repliesSuperseded

Changes in v4 from the cover letter

* Added blank line to separate declarations from statements
* unconditionally set referent member in base iterator in files backend

Patches in v4 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3refs: keep track of unresolved reference value in iteratorsrefs/files-backend.c refs/iterator.c refs/ref-cache.c +3 more+30 -51 reply
2/3refs: add referent to each_ref_fnbisect.c builtin/bisect.c builtin/checkout.c +45 more+78 -540 replies
3/3ref-filter: populate symref from iteratorref-filter.c+12 -50 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.

Read the v4 thread

Cover letter [PATCH v4 0/3] keep track of unresolved value of symbolic-ref in ref iterators

For reftable development, it's useful to be able to print out the direct value of a symbolic reference before resolution. This is currently possible with git-for-each-ref, but since the iterators do not keep track of the value of the symbolic ref, a separate call needs to be made each time making it inefficient.

Address this inefficiency by keeping track of the value of the symbolic reference in the ref iterator. This patch series also ends with a commit to use this value in the iterator through callbacks in ref-filter.c.

This series started with [1] but I decided to send a separate patch series since it is substantially different.

Benchmarking shows that with these changes, we experience a speedup in git-for-each-ref(1) on a repository with many symbolic refs:

$ hyperfine --warmup 5 "git for-each-ref --format='%(refname) %(objectname)
%(symref)'" "~/Projects/git/git for-each-ref --format='%(refname)
%(objectname) %(symref)'" Benchmark 1: git for-each-ref --format='%(refname)
%(objectname) %(symref)' Time (mean ± σ): 905.1 ms ± 13.2 ms [User: 56.3 ms,
System: 628.6 ms] Range (min … max): 893.4 ms … 936.9 ms 10 runs

Benchmark 2: ~/Projects/git/git for-each-ref --format='%(refname) %(objectname) %(symref)' Time (mean ± σ): 482.2 ms ± 26.4 ms [User: 34.7 ms,
System: 410.6 ms] Range (min … max): 459.4 ms … 541.8 ms 10 runs

Summary ~/Projects/git/git for-each-ref --format='%(refname) %(objectname) %(symref)' ran 1.88 ± 0.11 times faster than git for-each-ref --format='%(refname) %(objectname) %(symref)'

Changes since V3:

* Added blank line to separate declarations from statements
* unconditionally set referent member in base iterator in files backend

Changes since V2:

* Style tweaks
* Added a NEEDSWORK comment around fallback code
* Stay consistent in the meaning of a non-NULL referent member in the iterator

Changes since V1

* Use the return value from refs_resolve_ref_unsafe instead of using an out parameter

1. https://lore.kernel.org/git/[email protected]/

John Cai (3):
refs: keep track of unresolved reference value in iterators
refs: add referent to each_ref_fn
ref-filter: populate symref from iterator

 bisect.c                    |  3 ++-
 builtin/bisect.c            |  4 +++-
 builtin/checkout.c          |  2 +-
 builtin/describe.c          |  2 +-
 builtin/fetch.c             |  3 ++-
 builtin/fsck.c              |  4 ++--
 builtin/gc.c                |  1 +
 builtin/name-rev.c          |  2 +-
 builtin/pack-objects.c      |  5 +++--
 builtin/receive-pack.c      |  2 +-
 builtin/remote.c            |  4 +++-
 builtin/repack.c            |  1 +
 builtin/replace.c           |  1 +
 builtin/rev-parse.c         |  4 ++--
 builtin/show-branch.c       |  8 ++++----
 builtin/show-ref.c          |  3 ++-
 builtin/submodule--helper.c |  1 +
 builtin/worktree.c          |  1 +
 commit-graph.c              |  1 +
 delta-islands.c             |  2 +-
 fetch-pack.c                |  2 ++
 help.c                      |  2 +-
 http-backend.c              |  4 ++--
 log-tree.c                  |  2 +-
 ls-refs.c                   |  4 ++--
 midx-write.c                |  2 +-
 negotiator/default.c        |  2 +-
 negotiator/skipping.c       |  2 +-
 notes.c                     |  2 +-
 object-name.c               |  2 +-
 pseudo-merge.c              |  1 +
 reachable.c                 |  2 +-
 ref-filter.c                | 17 ++++++++++++-----
 reflog.c                    |  1 +
 refs.c                      | 14 +++++++-------
 refs.h                      |  2 +-
 refs/files-backend.c        | 17 +++++++++++++----
 refs/iterator.c             |  4 +++-
 refs/ref-cache.c            |  5 +++++
 refs/ref-cache.h            |  2 ++
 refs/refs-internal.h        |  1 +
 refs/reftable-backend.c     | 10 ++++++++--
 remote.c                    |  4 ++--
 replace-object.c            |  1 +
 revision.c                  |  2 +-
 server-info.c               |  2 +-
 shallow.c                   |  2 ++
 submodule.c                 |  2 ++
 t/helper/test-ref-store.c   |  2 +-
 upload-pack.c               |  8 ++++----
 walker.c                    |  1 +
 worktree.c                  |  2 +-
 52 files changed, 118 insertions(+), 62 deletions(-)

base-commit: 557ae147e6cdc9db121269b058c757ac5092f9c9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1712%2Fjohn-cai%2Fjc%2Fsymbolic-ref-in-iterator-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1712/john-cai/jc/symbolic-ref-in-iterator-v4
Pull-Request: https://github.com/git/git/pull/1712

Range-diff vs v3:

1: fc3defd9c47 ! 1: c4f5f5b7dd8 refs: keep track of unresolved reference value in iterators

     @@ refs/files-backend.c: static void loose_fill_ref_dir_regular_file(struct files_r
       {
       	struct object_id oid;
       	int flag;
     --
     --	if (!refs_resolve_ref_unsafe(&refs->base, refname, RESOLVE_REF_READING,
     --				     &oid, &flag)) {
      +	const char *referent = refs_resolve_ref_unsafe(&refs->base,
      +						       refname,
      +						       RESOLVE_REF_READING,
      +						       &oid, &flag);
     + 
     +-	if (!refs_resolve_ref_unsafe(&refs->base, refname, RESOLVE_REF_READING,
     +-				     &oid, &flag)) {
      +	if (!referent) {
       		oidclr(&oid, the_repository->hash_algo);
       		flag |= REF_ISBROKEN;
     @@ refs/files-backend.c: static int files_ref_iterator_advance(struct ref_iterator
       		iter->base.refname = iter->iter0->refname;
       		iter->base.oid = iter->iter0->oid;
       		iter->base.flags = iter->iter0->flags;
     -+		if (iter->iter0->flags & REF_ISSYMREF)
     -+			iter->base.referent = iter->iter0->referent;
     -+		else
     -+			iter->base.referent = NULL;
     ++		iter->base.referent = iter->iter0->referent;
      +
       		return ITER_OK;
       	}
     @@ refs/iterator.c: static int merge_ref_iterator_advance(struct ref_iterator *ref_
       			iter->base.flags = (*iter->current)->flags;
      
       ## refs/ref-cache.c ##
     -@@
     - #include "refs-internal.h"
     - #include "ref-cache.h"
     - #include "../iterator.h"
     -+#include "../strbuf.h"
     - 
     - void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry)
     - {
      @@ refs/ref-cache.c: struct ref_dir *get_ref_dir(struct ref_entry *entry)
       }
 2:  0b6e732ad7e = 2:  6cfac5e51f5 refs: add referent to each_ref_fn
 3:  83b70ab8287 = 3:  9f609f4d0f5 ref-filter: populate symref from iterator
Signature · 1 line
gitgitgadget