Make run_diff_index() use unpack_trees(), not read_tree()
A plain "git commit" would still run lstat() a lot more than necessary, because wt_status_print() would cause the index to be repeatedly flushed and re-read by wt_read_cache(), and that would cause the CE_UPTODATE bit to be lost, resulting in the files in the index being lstat'ed three times each. The reason why wt-status.c ended up invalidating and re-reading the cache multiple times was that it uses "run_diff_index()", which in turn uses "read_tree()" to populate the index with *both* the old index and the tree we want to compare against. So this patch re-writes run_diff_index() to not use read_tree(), but instead use "unpack_trees()" to diff the index to a tree. That, in turn, means that we don't need to modify the index itself, which then means that we don't need to invalidate it and re-read it! This, together with the lstat() optimizations, means that "git commit" on the kernel tree really only needs to lstat() the index entries once. That noticeably cuts down on the cached timings. 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 so it's a noticeable improvement in addition to being a nice conceptual cleanup (it's really not that pretty that "run_diff_index()" dirties the index!) Doing an "strace -c" on it also shows that as it cuts the number of lstat() calls by two thirds, it goes from being lstat()-limited to being limited by getdents() (which is the readdir system call): Before: % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 60.69 0.000704 0 69230 31 lstat 23.62 0.000274 0 5522 getdents 8.36 0.000097 0 5508 2638 open 2.59 0.000030 0 2869 close 2.50 0.000029 0 274 write 1.47 0.000017 0 2844 fstat After: % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 45.17 0.000276 0 5522 getdents 26.51 0.000162 0 23112 31 lstat 19.80 0.000121 0 5503 2638 open 4.91 0.000030 0 2864 close 1.48 0.000020 0 274 write 1.34 0.000018 0 2844 fstat ... It passes the test-suite for me, but this is another of one of those really core functions, and certainly pretty subtle, so.. NOTE! The Linux lstat() system call is really quite cheap when everything is cached, so the fact that this is quite noticeable on Linux is likely to mean that it is *much* more noticeable on other operating systems. I bet you'll see a much bigger performance improvement from this on Windows in particular. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Linus Torvalds
parent
eadb583134
commit
d1f2d7e8ca
151
diff-lib.c
151
diff-lib.c
@ -9,6 +9,7 @@
|
||||
#include "revision.h"
|
||||
#include "cache-tree.h"
|
||||
#include "path-list.h"
|
||||
#include "unpack-trees.h"
|
||||
|
||||
/*
|
||||
* diff-files
|
||||
@ -435,6 +436,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
|
||||
continue;
|
||||
}
|
||||
|
||||
if (ce_uptodate(ce))
|
||||
continue;
|
||||
if (lstat(ce->name, &st) < 0) {
|
||||
if (errno != ENOENT && errno != ENOTDIR) {
|
||||
perror(ce->name);
|
||||
@ -665,20 +668,133 @@ static void mark_merge_entries(void)
|
||||
}
|
||||
}
|
||||
|
||||
int run_diff_index(struct rev_info *revs, int cached)
|
||||
/*
|
||||
* This gets a mix of an existing index and a tree, one pathname entry
|
||||
* at a time. The index entry may be a single stage-0 one, but it could
|
||||
* also be multiple unmerged entries (in which case idx_pos/idx_nr will
|
||||
* give you the position and number of entries in the index).
|
||||
*/
|
||||
static void do_oneway_diff(struct unpack_trees_options *o,
|
||||
struct cache_entry *idx,
|
||||
struct cache_entry *tree,
|
||||
int idx_pos, int idx_nr)
|
||||
{
|
||||
int ret;
|
||||
struct object *ent;
|
||||
struct tree *tree;
|
||||
const char *tree_name;
|
||||
int match_missing = 0;
|
||||
struct rev_info *revs = o->unpack_data;
|
||||
int match_missing, cached;
|
||||
|
||||
/*
|
||||
* Backward compatibility wart - "diff-index -m" does
|
||||
* not mean "do not ignore merges", but totally different.
|
||||
* not mean "do not ignore merges", but "match_missing".
|
||||
*
|
||||
* But with the revision flag parsing, that's found in
|
||||
* "!revs->ignore_merges".
|
||||
*/
|
||||
if (!revs->ignore_merges)
|
||||
match_missing = 1;
|
||||
cached = o->index_only;
|
||||
match_missing = !revs->ignore_merges;
|
||||
|
||||
if (cached && idx && ce_stage(idx)) {
|
||||
if (tree)
|
||||
diff_unmerge(&revs->diffopt, idx->name, idx->ce_mode, idx->sha1);
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
* Something added to the tree?
|
||||
*/
|
||||
if (!tree) {
|
||||
show_new_file(revs, idx, cached, match_missing);
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
* Something removed from the tree?
|
||||
*/
|
||||
if (!idx) {
|
||||
diff_index_show_file(revs, "-", tree, tree->sha1, tree->ce_mode);
|
||||
return;
|
||||
}
|
||||
|
||||
/* Show difference between old and new */
|
||||
show_modified(revs, tree, idx, 1, cached, match_missing);
|
||||
}
|
||||
|
||||
/*
|
||||
* Count how many index entries go with the first one
|
||||
*/
|
||||
static inline int count_skip(const struct cache_entry *src, int pos)
|
||||
{
|
||||
int skip = 1;
|
||||
|
||||
/* We can only have multiple entries if the first one is not stage-0 */
|
||||
if (ce_stage(src)) {
|
||||
struct cache_entry **p = active_cache + pos;
|
||||
int namelen = ce_namelen(src);
|
||||
|
||||
for (;;) {
|
||||
const struct cache_entry *ce;
|
||||
pos++;
|
||||
if (pos >= active_nr)
|
||||
break;
|
||||
ce = *++p;
|
||||
if (ce_namelen(ce) != namelen)
|
||||
break;
|
||||
if (memcmp(ce->name, src->name, namelen))
|
||||
break;
|
||||
skip++;
|
||||
}
|
||||
}
|
||||
return skip;
|
||||
}
|
||||
|
||||
/*
|
||||
* The unpack_trees() interface is designed for merging, so
|
||||
* the different source entries are designed primarily for
|
||||
* the source trees, with the old index being really mainly
|
||||
* used for being replaced by the result.
|
||||
*
|
||||
* For diffing, the index is more important, and we only have a
|
||||
* single tree.
|
||||
*
|
||||
* We're supposed to return how many index entries we want to skip.
|
||||
*
|
||||
* This wrapper makes it all more readable, and takes care of all
|
||||
* the fairly complex unpack_trees() semantic requirements, including
|
||||
* the skipping, the path matching, the type conflict cases etc.
|
||||
*/
|
||||
static int oneway_diff(struct cache_entry **src,
|
||||
struct unpack_trees_options *o,
|
||||
int index_pos)
|
||||
{
|
||||
int skip = 0;
|
||||
struct cache_entry *idx = src[0];
|
||||
struct cache_entry *tree = src[1];
|
||||
struct rev_info *revs = o->unpack_data;
|
||||
|
||||
if (index_pos >= 0)
|
||||
skip = count_skip(idx, index_pos);
|
||||
|
||||
/*
|
||||
* Unpack-trees generates a DF/conflict entry if
|
||||
* there was a directory in the index and a tree
|
||||
* in the tree. From a diff standpoint, that's a
|
||||
* delete of the tree and a create of the file.
|
||||
*/
|
||||
if (tree == o->df_conflict_entry)
|
||||
tree = NULL;
|
||||
|
||||
if (ce_path_match(idx ? idx : tree, revs->prune_data))
|
||||
do_oneway_diff(o, idx, tree, index_pos, skip);
|
||||
|
||||
return skip;
|
||||
}
|
||||
|
||||
int run_diff_index(struct rev_info *revs, int cached)
|
||||
{
|
||||
struct object *ent;
|
||||
struct tree *tree;
|
||||
const char *tree_name;
|
||||
struct unpack_trees_options opts;
|
||||
struct tree_desc t;
|
||||
|
||||
mark_merge_entries();
|
||||
|
||||
@ -687,13 +803,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)
|
||||
|
||||
@ -25,6 +25,7 @@ struct unpack_trees_options {
|
||||
int merge_size;
|
||||
|
||||
struct cache_entry *df_conflict_entry;
|
||||
void *unpack_data;
|
||||
};
|
||||
|
||||
extern int unpack_trees(unsigned n, struct tree_desc *t,
|
||||
|
||||
10
wt-status.c
10
wt-status.c
@ -217,19 +217,12 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
|
||||
wt_status_print_trailer(s);
|
||||
}
|
||||
|
||||
static void wt_read_cache(struct wt_status *s)
|
||||
{
|
||||
discard_cache();
|
||||
read_cache_from(s->index_file);
|
||||
}
|
||||
|
||||
static void wt_status_print_initial(struct wt_status *s)
|
||||
{
|
||||
int i;
|
||||
struct strbuf buf;
|
||||
|
||||
strbuf_init(&buf, 0);
|
||||
wt_read_cache(s);
|
||||
if (active_nr) {
|
||||
s->commitable = 1;
|
||||
wt_status_print_cached_header(s);
|
||||
@ -256,7 +249,6 @@ static void wt_status_print_updated(struct wt_status *s)
|
||||
rev.diffopt.detect_rename = 1;
|
||||
rev.diffopt.rename_limit = 100;
|
||||
rev.diffopt.break_opt = 0;
|
||||
wt_read_cache(s);
|
||||
run_diff_index(&rev, 1);
|
||||
}
|
||||
|
||||
@ -268,7 +260,6 @@ static void wt_status_print_changed(struct wt_status *s)
|
||||
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
|
||||
rev.diffopt.format_callback = wt_status_print_changed_cb;
|
||||
rev.diffopt.format_callback_data = s;
|
||||
wt_read_cache(s);
|
||||
run_diff_files(&rev, 0);
|
||||
}
|
||||
|
||||
@ -335,7 +326,6 @@ static void wt_status_print_verbose(struct wt_status *s)
|
||||
setup_revisions(0, NULL, &rev, s->reference);
|
||||
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
|
||||
rev.diffopt.detect_rename = 1;
|
||||
wt_read_cache(s);
|
||||
run_diff_index(&rev, 1);
|
||||
|
||||
fflush(stdout);
|
||||
|
||||
Loading…
Reference in New Issue
Block a user