Re: [PATCH] Avoid running lstat(2) on the same cache entry.
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 20, 2008, 01:48 UTC
- Message-ID
- <alpine.LSU.1.00.0801200142170.5731@racer.site>
- In-Reply-To
- <alpine.LFD.1.00.0801191709380.2957@woody.linux-foundation.org>
Hi,
On Sat, 19 Jan 2008, Linus Torvalds wrote:
Show 13 quoted lines
> Best time before: > > [torvalds@woody linux]$ time git commit > /dev/null > real 0m0.399s > user 0m0.232s > sys 0m0.164s > > Best time after: > > [torvalds@woody linux]$ time git commit > /dev/null > real 0m0.254s > user 0m0.140s > sys 0m0.112s
Wow.
> I bet you'll see a much bigger performance improvement from this on > Windows in particular.
I bet so, too. Traditionally, filesystem calls are painfully slow on Windows.
But I cannot test before Monday, so I would not be mad if somebody else could perform some tests on Windows.
Now, the changes you made are quite a ways off of my current knowledge of git codepaths, so I cannot really comment. Although they look sane enough.
Show 26 quoted lines
> @@ -687,13 +732,20 @@ int run_diff_index(struct rev_info *revs, int cached)
> tree = parse_tree_indirect(ent->sha1);
> if (!tree)
> return error("bad tree object %s", tree_name);
> - if (read_tree(tree, 1, revs->prune_data))
> - return error("unable to read tree object %s", tree_name);
> - ret = diff_cache(revs, active_cache, active_nr, revs->prune_data,
> - cached, match_missing);
> +
> + memset(&opts, 0, sizeof(opts));
> + opts.head_idx = 1;
> + opts.index_only = cached;
> + opts.merge = 1;
> + opts.fn = oneway_diff;
> + opts.unpack_data = revs;
> +
> + init_tree_desc(&t, tree->buffer, tree->size);
> + unpack_trees(1, &t, &opts);
> +
> diffcore_std(&revs->diffopt);
> diff_flush(&revs->diffopt);
> - return ret;
> + return 0;
> }
>
> int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)What about do_diff_cache()? Shouldn't it get exactly the same treatment as run_diff_index()?
In which case, do_diff_cache() would not be needed anymore, too.
Show 9 quoted lines
> diff --git a/unpack-trees.h b/unpack-trees.h
> index 5517faa..197a004 100644
> --- a/unpack-trees.h
> +++ b/unpack-trees.h
> @@ -25,6 +25,7 @@ struct unpack_trees_options {
> int merge_size;
>
> struct cache_entry *df_conflict_entry;
> + void *unpack_data;Maybe this should be called "cb_data", in line with similar places in git.git?
Show 9 quoted lines
> diff --git a/wt-status.c b/wt-status.c > index c0c2472..a18ed11 100644 > --- a/wt-status.c > +++ b/wt-status.c > @@ -229,7 +229,6 @@ static void wt_status_print_initial(struct wt_status *s) > struct strbuf buf; > > strbuf_init(&buf, 0); > - wt_read_cache(s);
If I'm not mistaken, you removed all four call sites of wt_read_cache(), and it is static anyway, so it should be removed, no?
Ciao, Dscho "who will try to wrap his head around what read_tree() && diff_cache() are supposed to do"