diff options
| -rw-r--r-- | Documentation/technical/racy-git.txt | 193 | ||||
| -rw-r--r-- | Makefile | 6 | ||||
| -rw-r--r-- | check-racy.c | 28 | ||||
| -rw-r--r-- | read-cache.c | 69 |
4 files changed, 236 insertions, 60 deletions
diff --git a/Documentation/technical/racy-git.txt b/Documentation/technical/racy-git.txt new file mode 100644 index 0000000..7597d04 --- /dev/null +++ b/Documentation/technical/racy-git.txt @@ -0,0 +1,193 @@ +Use of index and Racy git problem +================================= + +Background +---------- + +The index is one of the most important data structure in git. +It represents a virtual working tree state by recording list of +paths and their object names and serves as a staging area to +write out the next tree object to be committed. The state is +"virtual" in the sense that it does not necessarily have to, and +often does not, match the files in the working tree. + +There are cases git needs to examine the differences between the +virtual working tree state in the index and the files in the +working tree. The most obvious case is when the user asks `git +diff` (or its low level implementation, `git diff-files`) or +`git-ls-files --modified`. In addition, git internally checks +if the files in the working tree is different from what are +recorded in the index to avoid stomping on local changes in them +during patch application, switching branches, and merging. + +In order to speed up this comparison between the files in the +working tree and the index entries, the index entries record the +information obtained from the filesystem via `lstat(2)` system +call when they were last updated. When checking if they differ, +git first runs `lstat(2)` on the files and compare the result +with this information (this is what was originally done by the +`ce_match_stat()` function, which the current code does in +`ce_match_stat_basic()` function). If some of these "cached +stat information" fields do not match, git can tell that the +files are modified without even looking at their contents. + +Note: not all members in `struct stat` obtained via `lstat(2)` +are used for this comparison. For example, `st_atime` obviously +is not useful. Currently, git compares the file type (regular +files vs symbolic links) and executable bits (only for regular +files) from `st_mode` member, `st_mtime` and `st_ctime` +timestamps, `st_uid`, `st_gid`, `st_ino`, and `st_size` members. +With a `USE_STDEV` compile-time option, `st_dev` is also +compared, but this is not enabled by default because this member +is not stable on network filesystems. With `USE_NSEC` +compile-time option, `st_mtim.tv_nsec` and `st_ctim.tv_nsec` +members are also compared, but this is not enabled by default +because the value of this member becomes meaningless once the +inode is evicted from the inode cache on filesystems that do not +store it on disk. + + +Racy git +-------- + +There is one slight problem with the optimization based on the +cached stat information. Consider this sequence: + + $ git update-index 'foo' + : modify 'foo' in-place without changing its size + +The first `update-index` computes the object name of the +contents of file `foo` and updates the index entry for `foo` +along with the `struct stat` information. If the modification +that follows it happens very fast so that the file's `st_mtime` +timestamp does not change, after this sequence, the cached stat +information the index entry records still exactly match what you +can obtain from the filesystem, but the file `foo` is modified. +This way, git can incorrectly think files in the working tree +are unmodified even though they actually are. This is called +the "racy git" problem (discovered by Pasky), and the entries +that appear clean when they may not be because of this problem +are called "racily clean". + +To avoid this problem, git does two things: + +. When the cached stat information says the file has not been + modified, and the `st_mtime` is the same as (or newer than) + the timestamp of the index file itself (which is the time `git + update-index foo` finished running in the above example), it + also compares the contents with the object registered in the + index entry to make sure they match. + +. When the index file is updated that contains racily clean + entries, cached `st_size` information is truncated to zero + before writing a new version of the index file. + +Because the index file itself is written after collecting all +the stat information from updated paths, `st_mtime` timestamp of +it is usually the same as or newer than any of the paths the +index contains. And no matter how quick the modification that +follows `git update-index foo` finishes, the resulting +`st_mtime` timestamp on `foo` cannot get the timestamp earlier +than the index file. Therefore, index entries that can be +racily clean are limited to the ones that have the same +timestamp as the index file itself. + +The callers that want to check if an index entry matches the +corresponding file in the working tree continue to call +`ce_match_stat()`, but with this change, `ce_match_stat()` uses +`ce_modified_check_fs()` to see if racily clean ones are +actually clean after comparing the cached stat information using +`ce_match_stat_basic()`. + +The problem the latter solves is this sequence: + + $ git update-index 'foo' + : modify 'foo' in-place without changing its size + : wait for enough time + $ git update-index 'bar' + +Without the latter, the timestamp of the index file gets a newer +value, and falsely clean entry `foo` would not be caught by the +timestamp comparison check done with the former logic anymore. +The latter makes sure that the cached stat information for `foo` +would never match with the file in the working tree, so later +checks by `ce_match_stat_basic()` would report the index entry +does not match the file and git does not have to fall back on more +expensive `ce_modified_check_fs()`. + + +Runtime penalty +--------------- + +The runtime penalty of falling back to `ce_modified_check_fs()` +from `ce_match_stat()` can be very expensive when there are many +racily clean entries. An obvious way to artificially create +this situation is to give the same timestamp to all the files in +the working tree in a large project, run `git update-index` on +them, and give the same timestamp to the index file: + + $ date >.datestamp + $ git ls-files | xargs touch -r .datestamp + $ git ls-files | git update-index --stdin + $ touch -r .datestamp .git/index + +This will make all index entries racily clean. The linux-2.6 +project, for example, there are over 20,000 files in the working +tree. On my Athron 64X2 3800+, after the above: + + $ /usr/bin/time git diff-files + 1.68user 0.54system 0:02.22elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k + 0inputs+0outputs (0major+67111minor)pagefaults 0swaps + $ git update-index MAINTAINERS + $ /usr/bin/time git diff-files + 0.02user 0.12system 0:00.14elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k + 0inputs+0outputs (0major+935minor)pagefaults 0swaps + +Running `git update-index` in the middle checked the racily +clean entries, and left the cached `st_mtime` for all the paths +intact because they were actually clean (so this step took about +the same amount of time as the first `git diff-files`). After +that, they are not racily clean anymore but are truly clean, so +the second invocation of `git diff-files` fully took advantage +of the cached stat information. + + +Avoiding runtime penalty +------------------------ + +In order to avoid the above runtime penalty, the recent "master" +branch (post 1.4.2) has a code that makes sure the index file +gets timestamp newer than the youngest files in the index when +there are many young files with the same timestamp as the +resulting index file would otherwise would have by waiting +before finishing writing the index file out. + +I suspect that in practice the situation where many paths in the +index are all racily clean is quite rare. The only code paths +that can record recent timestamp for large number of paths I +know of are: + +. Initial `git add .` of a large project. + +. `git checkout` of a large project from an empty index into an + unpopulated working tree. + +Note: switching branches with `git checkout` keeps the cached +stat information of existing working tree files that are the +same between the current branch and the new branch, which are +all older than the resulting index file, and they will not +become racily clean. Only the files that are actually checked +out can become racily clean. + +In a large project where raciness avoidance cost really matters, +however, the initial computation of all object names in the +index takes more than one second, and the index file is written +out after all that happens. Therefore the timestamp of the +index file will be more than one seconds later than the the +youngest file in the working tree. This means that in these +cases there actually will not be any racily clean entry in +the resulting index. + +So in summary I think we should not worry about avoiding the +runtime penalty and get rid of the "wait before finishing +writing" code out. @@ -195,7 +195,11 @@ PROGRAMS = \ git-update-server-info$X \ git-upload-pack$X git-verify-pack$X \ git-pack-redundant$X git-var$X \ - git-describe$X git-merge-tree$X git-blame$X git-imap-send$X + git-describe$X git-merge-tree$X git-blame$X git-imap-send$X \ + $(EXTRA_PROGRAMS) + +# Empty... +EXTRA_PROGRAMS = BUILT_INS = \ git-format-patch$X git-show$X git-whatchanged$X \ diff --git a/check-racy.c b/check-racy.c new file mode 100644 index 0000000..d6a08b4 --- /dev/null +++ b/check-racy.c @@ -0,0 +1,28 @@ +#include "cache.h" + +int main(int ac, char **av) +{ + int i; + int dirty, clean, racy; + + dirty = clean = racy = 0; + read_cache(); + for (i = 0; i < active_nr; i++) { + struct cache_entry *ce = active_cache[i]; + struct stat st; + + if (lstat(ce->name, &st)) { + error("lstat(%s): %s", ce->name, strerror(errno)); + continue; + } + + if (ce_match_stat(ce, &st, 0)) + dirty++; + else if (ce_match_stat(ce, &st, 2)) + racy++; + else + clean++; + } + printf("dirty %d, clean %d, racy %d\n", dirty, clean, racy); + return 0; +} diff --git a/read-cache.c b/read-cache.c index 3228ffb..6bec833 100644 --- a/read-cache.c +++ b/read-cache.c @@ -5,7 +5,6 @@ */ #include "cache.h" #include "cache-tree.h" -#include <time.h> /* Index extensions. * @@ -170,9 +169,11 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st) return changed; } -int ce_match_stat(struct cache_entry *ce, struct stat *st, int ignore_valid) +int ce_match_stat(struct cache_entry *ce, struct stat *st, int options) { unsigned int changed; + int ignore_valid = options & 01; + int assume_racy_is_modified = options & 02; /* * If it's marked as always valid in the index, it's @@ -201,8 +202,12 @@ int ce_match_stat(struct cache_entry *ce, struct stat *st, int ignore_valid) */ if (!changed && index_file_timestamp && - index_file_timestamp <= ntohl(ce->ce_mtime.sec)) - changed |= ce_modified_check_fs(ce, st); + index_file_timestamp <= ntohl(ce->ce_mtime.sec)) { + if (assume_racy_is_modified) + changed |= DATA_CHANGED; + else + changed |= ce_modified_check_fs(ce, st); + } return changed; } @@ -954,9 +959,7 @@ int write_cache(int newfd, struct cache_entry **cache, int entries) { SHA_CTX c; struct cache_header hdr; - int i, removed, recent; - struct stat st; - time_t now; + int i, removed; for (i = removed = 0; i < entries; i++) if (!cache[i]->ce_mode) @@ -994,57 +997,5 @@ int write_cache(int newfd, struct cache_entry **cache, int entries) return -1; } } - - /* - * To prevent later ce_match_stat() from always falling into - * check_fs(), if we have too many entries that can trigger - * racily clean check, we are better off delaying the return. - * We arbitrarily say if more than 20 paths or 25% of total - * paths are very new, we delay the return until the index - * file gets a new timestamp. - * - * NOTE! NOTE! NOTE! - * - * This assumes that nobody is touching the working tree while - * we are updating the index. - */ - - /* Make sure that the new index file has st_mtime - * that is current enough -- ce_write() batches the data - * so it might not have written anything yet. - */ - ce_write_flush(&c, newfd); - - now = fstat(newfd, &st) ? 0 : st.st_mtime; - if (now) { - recent = 0; - for (i = 0; i < entries; i++) { - struct cache_entry *ce = cache[i]; - time_t entry_time = (time_t) ntohl(ce->ce_mtime.sec); - if (!ce->ce_mode) - continue; - if (now && now <= entry_time) - recent++; - } - if (20 < recent && entries <= recent * 4) { -#if 0 - fprintf(stderr, "entries %d\n", entries); - fprintf(stderr, "recent %d\n", recent); - fprintf(stderr, "now %lu\n", now); -#endif - while (!fstat(newfd, &st) && st.st_mtime <= now) { - struct timespec rq, rm; - off_t where = lseek(newfd, 0, SEEK_CUR); - rq.tv_sec = 0; - rq.tv_nsec = 250000000; - nanosleep(&rq, &rm); - if ((where == (off_t) -1) || - (write(newfd, "", 1) != 1) || - (lseek(newfd, -1, SEEK_CUR) != where) || - ftruncate(newfd, where)) - break; - } - } - } return ce_flush(&c, newfd); } |