All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lea Wiemann <lewiemann@gmail.com>
To: Jakub Narebski <jnareb@gmail.com>
Cc: Rafael Garcia-Suarez <rgarciasuarez@gmail.com>,
	git@vger.kernel.org, Luben Tuikov <ltuikov@yahoo.com>
Subject: Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame
Date: Wed, 04 Jun 2008 01:11:27 +0200	[thread overview]
Message-ID: <4845CF9F.10604@gmail.com> (raw)
In-Reply-To: <200806032224.08714.jnareb@gmail.com>

Jakub Narebski wrote:
> I don't think %parent_commits hash is suitable for caching; it is only
> intermediate step, reducing number of git command calls (and forks) [...]
> 
> ATTENTION! This example shows where caching [parsed] data have problems 
> compared to front-end caching (caching output).

ATTENTION!  Could we please stop having this discussion?!  Your argument 
is completely bogus.  If the parent commit hashes are in cache, it's an 
almost zero-time cache lookup.  The only difference it might make 
compared front-end caching is the CPU time it takes to generate the 
page, and *I want to see benchmarks before I even start thinking about 
CPU*.  Okay?  Good, thanks.

Sorry I'm a little indignant, but you seem to be somehow trying to tell 
me what to implement, and that gets annoying after a while.  I don't 
mind your input, but at some point the discussion just doesn't go any 
further.

> Problems occur when we try to cache page with _streaming_ output, such 
> as blob view, blame view, diff part of commitdiff etc.

We can still stream backend-cache-backed data, though it's a little 
harder.  It's mostly a memory, not a performance issue though -- the 
only point where I think it actually would be performance-relevant is 
blame, and blame doesn't stream anyway (see below).

> By the way, if we agree that version %parent_commits is too intrusive 
> dusring GSoC 2008,

Oh, I don't mind, FTR.  It's not enough lines to matter.

>> 2) Major point: You're still forking a lot.  The Right Thing is to
>> condense everything into a single call
> 
> This is not a good solution for 'blame' view, which is generated "on the 
> fly", by streaming git-blame output via filter.

No, whether you have your "while <$fd>" loop or not doesn't make a 
difference.  Blame first calculates the whole blame and then dumps it 
out in zero-time, unless you use --incremental.  So there's no 
performance difference in getting all blame output and then dumping it 
out vs. reading and outputting it line-by-line.  And regarding memory, 
if your blame output doesn't fit into your RAM, you have different kinds 
of issues.

JFTR, I don't have any opinion about extending the porcelain output of 
git-blame (apart from the fact that happens to not be useful for gitweb 
for the reason I outlined in the previous paragraph).

-- Lea

  reply	other threads:[~2008-06-03 23:11 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-03 10:46 [PATCH] Avoid errors from git-rev-parse in gitweb blame Rafael Garcia-Suarez
2008-06-03 11:42 ` Lea Wiemann
2008-06-03 11:43 ` Jakub Narebski
2008-06-03 12:03   ` Rafael Garcia-Suarez
2008-06-03 12:45     ` Jakub Narebski
2008-06-03 13:00       ` Rafael Garcia-Suarez
2008-06-03 13:12         ` Jakub Narebski
2008-06-03 13:36           ` Rafael Garcia-Suarez
2008-06-03 14:14             ` Jakub Narebski
2008-06-03 14:40               ` Rafael Garcia-Suarez
2008-06-03 14:56                 ` Jakub Narebski
2008-06-03 15:07                   ` Rafael Garcia-Suarez
2008-06-03 17:50                     ` Jakub Narebski
2008-06-03 21:09                   ` Luben Tuikov
2008-06-03 21:03               ` Luben Tuikov
2008-06-03 20:35         ` Luben Tuikov
2008-06-03 21:31           ` Jakub Narebski
2008-06-04  5:58             ` Junio C Hamano
2008-06-04 14:03               ` Jakub Narebski
2008-06-05  6:07                 ` Junio C Hamano
2008-06-05  6:09                   ` [PATCH 1/2] git-blame: refactor code to emit "porcelain format" output Junio C Hamano
2008-06-06  9:22                     ` Jakub Narebski
2008-06-05  6:09                   ` [PATCH 2/2] blame: show "previous" information in --porcelain/--incremental format Junio C Hamano
2008-06-06  9:27                     ` Jakub Narebski
2008-06-06 15:17                       ` Junio C Hamano
2008-06-06 15:44                         ` Jakub Narebski
2008-06-06  0:26                   ` [PATCH] Avoid errors from git-rev-parse in gitweb blame Jakub Narebski
2008-06-04 22:24               ` Luben Tuikov
2008-06-03 14:24       ` Lea Wiemann
2008-06-03 20:24         ` Jakub Narebski
2008-06-03 23:11           ` Lea Wiemann [this message]
2008-06-04  0:11             ` Jakub Narebski
2008-06-04  0:39               ` Lea Wiemann
2008-06-04 12:31                 ` Jakub Narebski
2008-06-08 18:19             ` Lea Wiemann
2008-06-08 20:28               ` Jakub Narebski
2008-06-03 20:18   ` Luben Tuikov
2008-06-03 20:29     ` Jakub Narebski
2008-06-03 21:27       ` Luben Tuikov
2008-06-03 21:34         ` Jakub Narebski

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=4845CF9F.10604@gmail.com \
    --to=lewiemann@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=ltuikov@yahoo.com \
    --cc=rgarciasuarez@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.