git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: "René Scharfe" <l.s.r@web.de>, git@vger.kernel.org
Subject: [PATCH 0/4] finishing touches on jk/for-each-object-iteration
Date: Tue, 14 Aug 2018 14:13:59 -0400	[thread overview]
Message-ID: <20180814181358.GA26391@sigill.intra.peff.net> (raw)
In-Reply-To: <20180813184506.18232-1-jonathantanmy@google.com>

On Mon, Aug 13, 2018 at 11:45:06AM -0700, Jonathan Tan wrote:

> >   [1/7]: for_each_*_object: store flag definitions in a single location
> >   [2/7]: for_each_*_object: take flag arguments as enum
> >   [3/7]: for_each_*_object: give more comprehensive docstrings
> >   [4/7]: for_each_packed_object: support iterating in pack-order
> >   [5/7]: t1006: test cat-file --batch-all-objects with duplicates
> >   [6/7]: cat-file: rename batch_{loose,packed}_object callbacks
> >   [7/7]: cat-file: support "unordered" output for --batch-all-objects
> 
> Thanks for laying all the patches out so cleanly! All of them are:
> Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
> 
> Normally I would re-explain the patches to demonstrate that I understand
> them, but in this case, I think they are simple enough - patches 1, 2,
> 3, and 6 are refactorings that I agree with, patch 5 just makes a test
> more comprehensive, and patches 4 and 7 do what their commit messages
> say.
> 
> Stefan brought up the concern that cache.h is increasing in size, but I
> agree with the patch as written that it's probably best that we
> centralize all the flags somewhere, and we can deal with the location in
> a future patch.

Thanks for the review. Here are a few patches on top to deal with the
cache.h thing, as well as some optimizations that came out of discussing
oidset in another thread (I left out for now the "big" optimization of
moving oidset to a different data structure; that's complicated enough
to be dealt with on its own, I think).

The first patch here could arguably be squashed into the final patch of
the original series, but I'm OK with it either way.

  [1/4]: cat-file: use oidset check-and-insert
  [2/4]: cat-file: split batch "buf" into two variables
  [3/4]: cat-file: use a single strbuf for all output
  [4/4]: for_each_*_object: move declarations to object-store.h

 builtin/cat-file.c     | 43 +++++++++++---------
 builtin/prune-packed.c |  1 +
 cache.h                | 75 -----------------------------------
 object-store.h         | 90 ++++++++++++++++++++++++++++++++++++++++++
 packfile.h             | 20 ----------
 5 files changed, 116 insertions(+), 113 deletions(-)

-Peff

  reply	other threads:[~2018-08-14 18:14 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-10 23:07 [PATCH 0/7] speeding up cat-file by reordering object access Jeff King
2018-08-10 23:09 ` [PATCH 1/7] for_each_*_object: store flag definitions in a single location Jeff King
2018-08-10 23:27   ` Stefan Beller
2018-08-10 23:31     ` Jeff King
2018-08-10 23:33       ` Jeff King
2018-08-10 23:39         ` Stefan Beller
2018-08-11  0:33           ` Jeff King
2018-08-10 23:09 ` [PATCH 2/7] for_each_*_object: take flag arguments as enum Jeff King
2018-08-10 23:11 ` [PATCH 3/7] for_each_*_object: give more comprehensive docstrings Jeff King
2018-08-10 23:15 ` [PATCH 4/7] for_each_packed_object: support iterating in pack-order Jeff King
2018-08-15 13:28   ` Derrick Stolee
2018-08-16 17:36     ` Jeff King
2018-08-10 23:16 ` [PATCH 5/7] t1006: test cat-file --batch-all-objects with duplicates Jeff King
2018-08-10 23:17 ` [PATCH 6/7] cat-file: rename batch_{loose,packed}_object callbacks Jeff King
2018-08-10 23:24 ` [PATCH 7/7] cat-file: support "unordered" output for --batch-all-objects Jeff King
2018-08-13 18:45 ` [PATCH 0/7] speeding up cat-file by reordering object access Jonathan Tan
2018-08-14 18:13   ` Jeff King [this message]
2018-08-14 18:14     ` [PATCH 1/4] cat-file: use oidset check-and-insert Jeff King
2018-08-14 18:18     ` [PATCH 2/4] cat-file: split batch "buf" into two variables Jeff King
2018-08-14 18:20     ` [PATCH 3/4] cat-file: use a single strbuf for all output Jeff King
2018-08-14 19:30       ` René Scharfe
2018-08-14 19:39         ` Jeff King
2018-08-14 18:21     ` [PATCH 4/4] for_each_*_object: move declarations to object-store.h Jeff King
2018-08-15 14:05 ` [PATCH 0/7] speeding up cat-file by reordering object access Derrick Stolee
2018-08-16 17:39   ` Jeff King
2018-08-16 18:52     ` Junio C Hamano
2018-08-16 19:45       ` Jeff King

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=20180814181358.GA26391@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=l.s.r@web.de \
    /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).