From: Linus Torvalds <torvalds@linux-foundation.org>
To: Nicolas Pitre <nico@cam.org>
Cc: Morten Welinder <mwelinder@gmail.com>,
Junio C Hamano <junkio@cox.net>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 2/2] Implement a simple delta_base cache
Date: Sat, 17 Mar 2007 19:20:41 -0700 (PDT) [thread overview]
Message-ID: <Pine.LNX.4.64.0703171911120.6730@woody.linux-foundation.org> (raw)
In-Reply-To: <alpine.LFD.0.83.0703172200060.18328@xanadu.home>
On Sat, 17 Mar 2007, Nicolas Pitre wrote:
>
> Well... in my opinion it is the _current_ tree walker that is quite ugly
> and complex. It is always messier to parse strings than fixed width
> binary fields.
Sure. On the other hand, text is what made things easy to do initially,
and you're missing one *BIG* clue: you cannot remote the support without
losing compatibility with all traditional object formats.
So you have no choice. You need to support the text representation. As a
result, *your* code will now be way more ugly and messy.
The thing is, parsing some little text may sound expensive, but if the
expense is in finding the end of the string, we're doing really well.
In other words: the data structures are both simple and straightforward,
and the only reason strlen() shows up at all is:
- we pass strings around as just C strings, even when we know their
lengths. Prime example: look at tree-diff.c. And when you look at it,
realize that *for*every*single*strlen* in that file except for the very
last one (which is only used once per process for setup) we actually
know the string length from before, but we (well, *I*) decided that it
wasn't worth passing down as a parameter all the time.
- the simple parsing of the tree itself (which really isn't that
expensive - the real expense is bringing the data into the CPU cache,
but that's something we'd need to do *anyway*).
So I seriously suspect that you could get the strlen() overhead down from
that 16% pretty easily, but you'd have to pass the length of the "base"
string along all the time (and in the tree_entry cases you'd replace the
"strlen()" calls with a call to something like
static inline int tree_entry_len(const char *name, const unsigned char *sha1)
{
return (char *)sha1 - (char *)name - 1;
}
which will do it for you).
But what you're ignoring here is that "16%" may sound like a huge deal,
but it's 16% of somethng that takes 1 second, and that other SCM's cannot
do AT ALL.
Linus
next prev parent reply other threads:[~2007-03-18 2:21 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-03-16 1:04 cleaner/better zlib sources? Linus Torvalds
2007-03-16 1:10 ` Shawn O. Pearce
2007-03-16 1:11 ` Jeff Garzik
2007-03-16 1:14 ` Matt Mackall
2007-03-16 1:46 ` Linus Torvalds
2007-03-16 1:54 ` Linus Torvalds
2007-03-16 2:43 ` Davide Libenzi
2007-03-16 2:56 ` Linus Torvalds
2007-03-16 3:16 ` Davide Libenzi
2007-03-16 16:21 ` Linus Torvalds
2007-03-16 16:24 ` Davide Libenzi
2007-03-16 16:35 ` Linus Torvalds
2007-03-16 19:21 ` Davide Libenzi
2007-03-17 0:01 ` Linus Torvalds
2007-03-17 1:11 ` Linus Torvalds
2007-03-17 3:28 ` Nicolas Pitre
2007-03-17 5:19 ` Shawn O. Pearce
2007-03-17 17:55 ` Linus Torvalds
2007-03-17 19:40 ` Linus Torvalds
2007-03-17 19:42 ` [PATCH 1/2] Make trivial wrapper functions around delta base generation and freeing Linus Torvalds
2007-03-17 19:44 ` [PATCH 2/2] Implement a simple delta_base cache Linus Torvalds
2007-03-17 21:45 ` Linus Torvalds
2007-03-17 22:37 ` Junio C Hamano
2007-03-17 23:09 ` Linus Torvalds
2007-03-17 23:54 ` Linus Torvalds
2007-03-18 1:13 ` Nicolas Pitre
2007-03-18 7:47 ` Junio C Hamano
2007-03-17 23:12 ` Junio C Hamano
2007-03-17 23:24 ` Linus Torvalds
2007-03-17 23:52 ` Jon Smirl
2007-03-18 1:14 ` Morten Welinder
2007-03-18 1:29 ` Linus Torvalds
2007-03-18 1:38 ` Nicolas Pitre
2007-03-18 1:55 ` Linus Torvalds
2007-03-18 2:03 ` Nicolas Pitre
2007-03-18 2:20 ` Linus Torvalds [this message]
2007-03-18 3:00 ` Nicolas Pitre
2007-03-18 3:31 ` Linus Torvalds
2007-03-18 5:30 ` Julian Phillips
2007-03-18 17:23 ` Linus Torvalds
2007-03-18 10:53 ` Robin Rosenberg
2007-03-18 17:34 ` Linus Torvalds
2007-03-18 18:29 ` Robin Rosenberg
2007-03-18 21:25 ` Shawn O. Pearce
2007-03-19 13:16 ` David Brodsky
2007-03-20 6:35 ` Robin Rosenberg
2007-03-20 9:13 ` David Brodsky
2007-03-21 2:37 ` Linus Torvalds
2007-03-21 2:54 ` Nicolas Pitre
2007-03-18 3:06 ` [PATCH 3/2] Avoid unnecessary strlen() calls Linus Torvalds
2007-03-18 9:45 ` Junio C Hamano
2007-03-18 15:54 ` Linus Torvalds
2007-03-18 15:57 ` Linus Torvalds
2007-03-18 21:38 ` Shawn O. Pearce
2007-03-18 21:48 ` Linus Torvalds
2007-03-20 3:05 ` Johannes Schindelin
2007-03-20 3:29 ` Shawn O. Pearce
2007-03-20 3:40 ` Shawn O. Pearce
2007-03-20 4:11 ` Linus Torvalds
2007-03-20 4:18 ` Shawn O. Pearce
2007-03-20 4:45 ` Linus Torvalds
2007-03-20 5:44 ` Junio C Hamano
2007-03-20 3:16 ` Junio C Hamano
2007-03-20 4:31 ` Linus Torvalds
2007-03-20 4:39 ` Shawn O. Pearce
2007-03-20 4:57 ` Linus Torvalds
2007-03-18 1:44 ` [PATCH 2/2] Implement a simple delta_base cache Linus Torvalds
2007-03-18 6:28 ` Avi Kivity
2007-03-17 22:44 ` Linus Torvalds
2007-03-16 16:35 ` cleaner/better zlib sources? Jeff Garzik
2007-03-16 16:42 ` Matt Mackall
2007-03-16 16:51 ` Linus Torvalds
2007-03-16 17:12 ` Nicolas Pitre
2007-03-16 23:22 ` Shawn O. Pearce
2007-03-16 17:06 ` Nicolas Pitre
2007-03-16 17:51 ` Linus Torvalds
2007-03-16 18:09 ` Nicolas Pitre
2007-03-16 1:33 ` Davide Libenzi
2007-03-16 2:06 ` Davide Libenzi
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=Pine.LNX.4.64.0703171911120.6730@woody.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=git@vger.kernel.org \
--cc=junkio@cox.net \
--cc=mwelinder@gmail.com \
--cc=nico@cam.org \
/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).