git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marco Costalba" <mcostalba@gmail.com>
To: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH] Add --log-size to git log to print message size
Date: Fri, 20 Jul 2007 22:49:17 +0200	[thread overview]
Message-ID: <e5bfff550707201349i52363837o739d27c6c4dc3ec8@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0707201939170.14781@racer.site>

On 7/20/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Fri, 20 Jul 2007, Marco Costalba wrote:
>
> > This is take 3 of this patch. In this case has been clearly added that
> > diff content size is not included.
>
> Two concerns and a half:
>
> - if you do not include the diff content size, don't you need to parse the
>   output anyway?
>

First, dif content is included only when retrieving file history and
not to show revisions at startup that is the main use.

Second, also with diff content I don't parse the message because I
jump directly to the beginning of the diff and, yes, from there I
check for delimiting '\0', so I would say also in that case we have a
speedup, but it's not very important because we are talking of speed
up when you read thousands of revisions, few hundred (as typical in
file histories) are not a problem.

Also because with -p option git log becomes a real bottleneck.


> - what do you do if the underlying Git does not support --log-size? Exit?
>

If for "not support" you mean that underlying Git sets size at zero
(as allowed by specifications) this is of course handled: qgit falls
back on legacy '\0' search. BTW the currently  published version does
exactly that, it has already the shortcut logic, but falls back always
on zero search because the modified git is _currently_ only on my hard
disk.


Otherwise, if you mean that git version is too old, in this case this
is caught by the git version check done always at startup.

Indeed I plan to add this feature only to new qgit4, will not be back
ported to stable qgit-1.5.6

> - Would it not be much quicker to read the rev-list, and read messages
>   only on demand?
>

That's exactly what we are talking about ;-)

 All the output of git-log (no more git-rev-list) is stored in memory
as a raw big binary blob, for quick retrieval, i.e. to avoid calling
an external git command process, but messages are not parsed until are
used. So _this_ is exactly the point of all this patch.

If you mean using git-rev-list without --header options then , also
not considering that currently we use git-log, the speed up is very
small and when you have to show something to the user you always need
to run a git command.

Perhaps this could be accepted (perhaps not ;-) for mouse browsing
through revisions, but what about sub string searching as example on
author or log title fields? probably it would became super slow also
for lists of only few hundred revs.


> Ciao,
> Dscho

Ciao
Marco

      reply	other threads:[~2007-07-20 20:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-20 18:15 [PATCH] Add --log-size to git log to print message size Marco Costalba
2007-07-20 18:41 ` Johannes Schindelin
2007-07-20 20:49   ` Marco Costalba [this message]

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=e5bfff550707201349i52363837o739d27c6c4dc3ec8@mail.gmail.com \
    --to=mcostalba@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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).