On Sat, Sep 08, 2007 at 11:59:31AM +0000, David Kastrup wrote:
Pierre Habouzit [off-list ref] writes:
quoted
Also remove the "len" parameter, as:
(1) it was used as a max boundary, and every caller used ~0u
(2) we check for final NUL no matter what, so it doesn't help for speed.
That sounds like a change that makes improvement of callers impossible
when it is found out that it leads to a performance issue. Is it only
the pretty-print that is affected?
I removed the "len" argument of pretty_print_commit and all the sub
pp_* functions it uses. This argument was supposed to tell which size
the commit message you want to read to format the pretty printing.
It leads to a lot of code that works like this:
if (position < len || *msg) /* test if len is overflowed or if we are
at the end of the string */
break;
This impose us to maintain the len of the message while we make the
"msg" pointer progress, and so on. And it's a limit of what to read in the
commit message.
Seeing where pretty_print_commit is used:
builtin-branch.c: pretty_print_commit(CMIT_FMT_ONELINE, commit,
builtin-log.c: pretty_print_commit(CMIT_FMT_ONELINE, commit,
builtin-rev-list.c: pretty_print_commit(revs.commit_format, commit,
builtin-show-branch.c: pretty_print_commit(CMIT_FMT_ONELINE, commit,
commit.c:void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
log-tree.c: pretty_print_commit(opt->commit_format, commit, &msgbuf,
I assume that the user would not be very pleased if we decide to crop
his commits logs when he uses git-log. And given that git log in the
linux repository on my laptop takes:
2,13s user 0,06s system 99% cpu 2,213 total
when the repo is hot ... I hardly think it can ever be a performance
issue :)
But if people disagree I can refactor a patch with the "len" argument
kept.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
On sam, sep 08, 2007 at 11:59:31 +0000, David Kastrup wrote:
Pierre Habouzit [off-list ref] writes:
quoted
Also remove the "len" parameter, as:
(1) it was used as a max boundary, and every caller used ~0u
(2) we check for final NUL no matter what, so it doesn't help for speed.
That sounds like a change that makes improvement of callers impossible
when it is found out that it leads to a performance issue. Is it only
the pretty-print that is affected?
Speaking of performance, here is a small interesting bench:
$ for i in `seq 1 10`; do time git-log >| /dev/null; done
git-log >| /dev/null 2,12s user 0,08s system 99% cpu 2,205 total
git-log >| /dev/null 2,14s user 0,06s system 99% cpu 2,208 total
git-log >| /dev/null 2,12s user 0,08s system 99% cpu 2,205 total
git-log >| /dev/null 2,10s user 0,10s system 99% cpu 2,209 total
git-log >| /dev/null 2,14s user 0,07s system 99% cpu 2,211 total
git-log >| /dev/null 2,11s user 0,09s system 99% cpu 2,210 total
git-log >| /dev/null 2,15s user 0,05s system 99% cpu 2,213 total
git-log >| /dev/null 2,12s user 0,07s system 99% cpu 2,203 total
git-log >| /dev/null 2,13s user 0,06s system 99% cpu 2,204 total
git-log >| /dev/null 2,17s user 0,09s system 100% cpu 2,254 total
$ for i in `seq 1 10`; do time ~/dev/scm/git/git-log >| /dev/null; done
~/dev/scm/git/git-log >| /dev/null 2,06s user 0,11s system 99% cpu 2,185 total
~/dev/scm/git/git-log >| /dev/null 2,11s user 0,07s system 99% cpu 2,192 total
~/dev/scm/git/git-log >| /dev/null 2,10s user 0,10s system 99% cpu 2,207 total
~/dev/scm/git/git-log >| /dev/null 2,10s user 0,08s system 99% cpu 2,188 total
~/dev/scm/git/git-log >| /dev/null 2,10s user 0,08s system 99% cpu 2,187 total
~/dev/scm/git/git-log >| /dev/null 2,10s user 0,10s system 100% cpu 2,196 total
~/dev/scm/git/git-log >| /dev/null 2,08s user 0,11s system 99% cpu 2,195 total
~/dev/scm/git/git-log >| /dev/null 2,11s user 0,08s system 99% cpu 2,193 total
~/dev/scm/git/git-log >| /dev/null 2,10s user 0,08s system 100% cpu 2,188 total
~/dev/scm/git/git-log >| /dev/null 2,12s user 0,07s system 98% cpu 2,213 total
$ git rev-list --all | wc -l
64271
The underling repository is a not so old linux-2.6 git repository.
git-log is the default unstable git-log, (a pristine 1.5.3.1,
pre-new-strbuf).
~/dev/scm/git/git-log is obviously the new one with the whole strbuf
patch series applied. Strbufs definitely made the code more readable (at
least it's my impression), here is the proof that it did not affected an
inch from performance (we even seem to have a marginal 0.5% gain in
performance ;p).
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org