public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff King <peff@peff•net>
To: Olga Telezhnaya <olyatelezhnaya@gmail•com>
Cc: git <git@vger•kernel.org>, Christian Couder <christian.couder@gmail•com>
Subject: Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
Date: Thu, 28 Feb 2019 16:41:12 -0500	[thread overview]
Message-ID: <20190228214112.GK12723@sigill.intra.peff.net> (raw)
In-Reply-To: <CAL21BmnVkKtYWa1cRL1EJAwtchGcVUzhu0136AuV8uXAi5Kuew@mail.gmail.com>

On Fri, Feb 22, 2019 at 06:50:06PM +0300, Olga Telezhnaya wrote:

> It was a long way for me, I got older (by 1 year) and smarter
> (hopefully), and maybe I will finish my Outreachy Internship task for
> now. (I am doing it just for one year and a half, that's OK)

Welcome back!

Sorry to be a bit slow on the review. I've read through and commented on
patch 10. Some of my comments were "I'll have to see how this plays out
later in the series", so you may want to hold off on responding until I
read the rest. :)

> If serious:
> In this patch we remove cat-file formatting logic and reuse ref-filter
> logic there. As a positive side effect, cat-file now has many new
> formatting tokens (all from ref-filter formatting), including deref
> (like %(*objectsize:disk)). I have already tried to do this task one
> year ago, and it was bad attempt. I feel that today's patch is much
> better.

I'm still concerned that this is going to regress the performance of
cat-file noticeably without some big cleanups in ref-filter. Here are
timings on linux.git before and after your patches:

  [before]
  $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null
  real	0m16.602s
  user	0m15.545s
  sys	0m0.495s

  [after]
  $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null
  real	0m27.301s
  user	0m24.549s
  sys	0m2.752s

I don't think that's anything particularly wrong with your patches. It's
the existing strategy of ref-filter (in particular how it is very eager
to allocate lots of separate strings). And it may be too early to switch
cat-file over to it.

> I also have a question about site https://git-scm.com/docs/
> I thought it is updated automatically based on Documentation folder in
> the project, but it is not true. I edited docs for for-each-ref in
> December, I still see my patch in master, but for-each-ref docs in
> git-csm is outdated. Is it OK?

Yeah, as Eric noted, we only build docs for the tagged releases. In
theory it would be easy to just build the tip of master nightly, but the
data model for the site would need quite a bit of adjustment.

-Peff

  parent reply	other threads:[~2019-02-28 21:41 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-22 15:50 [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter Olga Telezhnaya
2019-02-22 16:05 ` [PATCH RFC 01/20] cat-file: reuse struct ref_format Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 13/20] cat-file: rewrite print_object_or_die Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 10/20] cat-file: inline stream_blob Olga Telezhnaya
2019-02-28 21:33     ` Jeff King
2019-02-22 16:05   ` [PATCH RFC 20/20] cat-file: update docs Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 11/20] cat-file: move filter_object to diff.c Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 04/20] for-each-ref: tests for new atom %(rest) added Olga Telezhnaya
2019-02-28 21:11     ` Jeff King
2019-03-01  6:10       ` Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 16/20] for-each-ref: tests for new atom %(raw) added Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 15/20] ref-filter: add raw formatting option Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 07/20] cat-file: remove skip_object_info Olga Telezhnaya
2019-02-28 21:26     ` Jeff King
2019-02-22 16:05   ` [PATCH RFC 05/20] cat-file: remove split_on_whitespace Olga Telezhnaya
2019-02-28 21:22     ` Jeff King
2019-02-22 16:05   ` [PATCH RFC 06/20] cat-file: remove mark_query from expand_data Olga Telezhnaya
2019-02-28 21:25     ` Jeff King
2019-03-03  9:41     ` Christian Couder
2019-02-22 16:05   ` [PATCH RFC 19/20] cat-file: tests for new atoms added Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 14/20] cat-file: move print_object_or_die to ref-filter Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 02/20] ref-filter: rename field in ref_array_item stuct Olga Telezhnaya
2019-02-28 21:06     ` Jeff King
2019-02-22 16:05   ` [PATCH RFC 03/20] ref-filter: add rest formatting option Olga Telezhnaya
2019-02-28 21:07     ` Jeff King
2019-02-22 16:05   ` [PATCH RFC 09/20] ref-filter: make expand_data global Olga Telezhnaya
2019-02-28 21:30     ` Jeff King
2019-02-22 16:05   ` [PATCH RFC 17/20] cat-file: reuse ref-filter formatting logic Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 08/20] cat-file: remove rest from expand_data Olga Telezhnaya
2019-02-28 21:27     ` Jeff King
2019-02-22 16:05   ` [PATCH RFC 12/20] cat-file: remove batch_write function Olga Telezhnaya
2019-02-22 16:05   ` [PATCH RFC 18/20] cat-file: get rid of expand_data Olga Telezhnaya
2019-02-28 21:04   ` [PATCH RFC 01/20] cat-file: reuse struct ref_format Jeff King
2019-02-22 16:09 ` [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter Eric Sunshine
2019-02-22 16:19   ` Olga Telezhnaya
2019-02-28 21:41 ` Jeff King [this message]
2019-03-01  6:16   ` Olga Telezhnaya
2019-02-28 21:43 ` Jeff King
2019-03-01  6:17   ` Olga Telezhnaya
2019-03-03  1:21   ` Junio C Hamano

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=20190228214112.GK12723@sigill.intra.peff.net \
    --to=peff@peff$(echo .)net \
    --cc=christian.couder@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=olyatelezhnaya@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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