Skip to content

Commit 8e3abd4

Browse files
author
Junio C Hamano
committed
Merge branch 'jc/racy'
* jc/racy: Remove the "delay writing to avoid runtime penalty of racy-git avoidance" Add check program "git-check-racy" Documentation/technical/racy-git.txt avoid nanosleep(2)
2 parents 500a999 + 0fc82cf commit 8e3abd4

File tree

4 files changed

+236
-60
lines changed

4 files changed

+236
-60
lines changed

Documentation/technical/racy-git.txt

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
Use of index and Racy git problem
2+
=================================
3+
4+
Background
5+
----------
6+
7+
The index is one of the most important data structure in git.
8+
It represents a virtual working tree state by recording list of
9+
paths and their object names and serves as a staging area to
10+
write out the next tree object to be committed. The state is
11+
"virtual" in the sense that it does not necessarily have to, and
12+
often does not, match the files in the working tree.
13+
14+
There are cases git needs to examine the differences between the
15+
virtual working tree state in the index and the files in the
16+
working tree. The most obvious case is when the user asks `git
17+
diff` (or its low level implementation, `git diff-files`) or
18+
`git-ls-files --modified`. In addition, git internally checks
19+
if the files in the working tree is different from what are
20+
recorded in the index to avoid stomping on local changes in them
21+
during patch application, switching branches, and merging.
22+
23+
In order to speed up this comparison between the files in the
24+
working tree and the index entries, the index entries record the
25+
information obtained from the filesystem via `lstat(2)` system
26+
call when they were last updated. When checking if they differ,
27+
git first runs `lstat(2)` on the files and compare the result
28+
with this information (this is what was originally done by the
29+
`ce_match_stat()` function, which the current code does in
30+
`ce_match_stat_basic()` function). If some of these "cached
31+
stat information" fields do not match, git can tell that the
32+
files are modified without even looking at their contents.
33+
34+
Note: not all members in `struct stat` obtained via `lstat(2)`
35+
are used for this comparison. For example, `st_atime` obviously
36+
is not useful. Currently, git compares the file type (regular
37+
files vs symbolic links) and executable bits (only for regular
38+
files) from `st_mode` member, `st_mtime` and `st_ctime`
39+
timestamps, `st_uid`, `st_gid`, `st_ino`, and `st_size` members.
40+
With a `USE_STDEV` compile-time option, `st_dev` is also
41+
compared, but this is not enabled by default because this member
42+
is not stable on network filesystems. With `USE_NSEC`
43+
compile-time option, `st_mtim.tv_nsec` and `st_ctim.tv_nsec`
44+
members are also compared, but this is not enabled by default
45+
because the value of this member becomes meaningless once the
46+
inode is evicted from the inode cache on filesystems that do not
47+
store it on disk.
48+
49+
50+
Racy git
51+
--------
52+
53+
There is one slight problem with the optimization based on the
54+
cached stat information. Consider this sequence:
55+
56+
$ git update-index 'foo'
57+
: modify 'foo' in-place without changing its size
58+
59+
The first `update-index` computes the object name of the
60+
contents of file `foo` and updates the index entry for `foo`
61+
along with the `struct stat` information. If the modification
62+
that follows it happens very fast so that the file's `st_mtime`
63+
timestamp does not change, after this sequence, the cached stat
64+
information the index entry records still exactly match what you
65+
can obtain from the filesystem, but the file `foo` is modified.
66+
This way, git can incorrectly think files in the working tree
67+
are unmodified even though they actually are. This is called
68+
the "racy git" problem (discovered by Pasky), and the entries
69+
that appear clean when they may not be because of this problem
70+
are called "racily clean".
71+
72+
To avoid this problem, git does two things:
73+
74+
. When the cached stat information says the file has not been
75+
modified, and the `st_mtime` is the same as (or newer than)
76+
the timestamp of the index file itself (which is the time `git
77+
update-index foo` finished running in the above example), it
78+
also compares the contents with the object registered in the
79+
index entry to make sure they match.
80+
81+
. When the index file is updated that contains racily clean
82+
entries, cached `st_size` information is truncated to zero
83+
before writing a new version of the index file.
84+
85+
Because the index file itself is written after collecting all
86+
the stat information from updated paths, `st_mtime` timestamp of
87+
it is usually the same as or newer than any of the paths the
88+
index contains. And no matter how quick the modification that
89+
follows `git update-index foo` finishes, the resulting
90+
`st_mtime` timestamp on `foo` cannot get the timestamp earlier
91+
than the index file. Therefore, index entries that can be
92+
racily clean are limited to the ones that have the same
93+
timestamp as the index file itself.
94+
95+
The callers that want to check if an index entry matches the
96+
corresponding file in the working tree continue to call
97+
`ce_match_stat()`, but with this change, `ce_match_stat()` uses
98+
`ce_modified_check_fs()` to see if racily clean ones are
99+
actually clean after comparing the cached stat information using
100+
`ce_match_stat_basic()`.
101+
102+
The problem the latter solves is this sequence:
103+
104+
$ git update-index 'foo'
105+
: modify 'foo' in-place without changing its size
106+
: wait for enough time
107+
$ git update-index 'bar'
108+
109+
Without the latter, the timestamp of the index file gets a newer
110+
value, and falsely clean entry `foo` would not be caught by the
111+
timestamp comparison check done with the former logic anymore.
112+
The latter makes sure that the cached stat information for `foo`
113+
would never match with the file in the working tree, so later
114+
checks by `ce_match_stat_basic()` would report the index entry
115+
does not match the file and git does not have to fall back on more
116+
expensive `ce_modified_check_fs()`.
117+
118+
119+
Runtime penalty
120+
---------------
121+
122+
The runtime penalty of falling back to `ce_modified_check_fs()`
123+
from `ce_match_stat()` can be very expensive when there are many
124+
racily clean entries. An obvious way to artificially create
125+
this situation is to give the same timestamp to all the files in
126+
the working tree in a large project, run `git update-index` on
127+
them, and give the same timestamp to the index file:
128+
129+
$ date >.datestamp
130+
$ git ls-files | xargs touch -r .datestamp
131+
$ git ls-files | git update-index --stdin
132+
$ touch -r .datestamp .git/index
133+
134+
This will make all index entries racily clean. The linux-2.6
135+
project, for example, there are over 20,000 files in the working
136+
tree. On my Athron 64X2 3800+, after the above:
137+
138+
$ /usr/bin/time git diff-files
139+
1.68user 0.54system 0:02.22elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
140+
0inputs+0outputs (0major+67111minor)pagefaults 0swaps
141+
$ git update-index MAINTAINERS
142+
$ /usr/bin/time git diff-files
143+
0.02user 0.12system 0:00.14elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
144+
0inputs+0outputs (0major+935minor)pagefaults 0swaps
145+
146+
Running `git update-index` in the middle checked the racily
147+
clean entries, and left the cached `st_mtime` for all the paths
148+
intact because they were actually clean (so this step took about
149+
the same amount of time as the first `git diff-files`). After
150+
that, they are not racily clean anymore but are truly clean, so
151+
the second invocation of `git diff-files` fully took advantage
152+
of the cached stat information.
153+
154+
155+
Avoiding runtime penalty
156+
------------------------
157+
158+
In order to avoid the above runtime penalty, the recent "master"
159+
branch (post 1.4.2) has a code that makes sure the index file
160+
gets timestamp newer than the youngest files in the index when
161+
there are many young files with the same timestamp as the
162+
resulting index file would otherwise would have by waiting
163+
before finishing writing the index file out.
164+
165+
I suspect that in practice the situation where many paths in the
166+
index are all racily clean is quite rare. The only code paths
167+
that can record recent timestamp for large number of paths I
168+
know of are:
169+
170+
. Initial `git add .` of a large project.
171+
172+
. `git checkout` of a large project from an empty index into an
173+
unpopulated working tree.
174+
175+
Note: switching branches with `git checkout` keeps the cached
176+
stat information of existing working tree files that are the
177+
same between the current branch and the new branch, which are
178+
all older than the resulting index file, and they will not
179+
become racily clean. Only the files that are actually checked
180+
out can become racily clean.
181+
182+
In a large project where raciness avoidance cost really matters,
183+
however, the initial computation of all object names in the
184+
index takes more than one second, and the index file is written
185+
out after all that happens. Therefore the timestamp of the
186+
index file will be more than one seconds later than the the
187+
youngest file in the working tree. This means that in these
188+
cases there actually will not be any racily clean entry in
189+
the resulting index.
190+
191+
So in summary I think we should not worry about avoiding the
192+
runtime penalty and get rid of the "wait before finishing
193+
writing" code out.

Makefile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,11 @@ PROGRAMS = \
195195
git-update-server-info$X \
196196
git-upload-pack$X git-verify-pack$X \
197197
git-pack-redundant$X git-var$X \
198-
git-describe$X git-merge-tree$X git-blame$X git-imap-send$X
198+
git-describe$X git-merge-tree$X git-blame$X git-imap-send$X \
199+
$(EXTRA_PROGRAMS)
200+
201+
# Empty...
202+
EXTRA_PROGRAMS =
199203

200204
BUILT_INS = \
201205
git-format-patch$X git-show$X git-whatchanged$X \

check-racy.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#include "cache.h"
2+
3+
int main(int ac, char **av)
4+
{
5+
int i;
6+
int dirty, clean, racy;
7+
8+
dirty = clean = racy = 0;
9+
read_cache();
10+
for (i = 0; i < active_nr; i++) {
11+
struct cache_entry *ce = active_cache[i];
12+
struct stat st;
13+
14+
if (lstat(ce->name, &st)) {
15+
error("lstat(%s): %s", ce->name, strerror(errno));
16+
continue;
17+
}
18+
19+
if (ce_match_stat(ce, &st, 0))
20+
dirty++;
21+
else if (ce_match_stat(ce, &st, 2))
22+
racy++;
23+
else
24+
clean++;
25+
}
26+
printf("dirty %d, clean %d, racy %d\n", dirty, clean, racy);
27+
return 0;
28+
}

read-cache.c

Lines changed: 10 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66
#include "cache.h"
77
#include "cache-tree.h"
8-
#include <time.h>
98

109
/* Index extensions.
1110
*
@@ -170,9 +169,11 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
170169
return changed;
171170
}
172171

173-
int ce_match_stat(struct cache_entry *ce, struct stat *st, int ignore_valid)
172+
int ce_match_stat(struct cache_entry *ce, struct stat *st, int options)
174173
{
175174
unsigned int changed;
175+
int ignore_valid = options & 01;
176+
int assume_racy_is_modified = options & 02;
176177

177178
/*
178179
* 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)
201202
*/
202203
if (!changed &&
203204
index_file_timestamp &&
204-
index_file_timestamp <= ntohl(ce->ce_mtime.sec))
205-
changed |= ce_modified_check_fs(ce, st);
205+
index_file_timestamp <= ntohl(ce->ce_mtime.sec)) {
206+
if (assume_racy_is_modified)
207+
changed |= DATA_CHANGED;
208+
else
209+
changed |= ce_modified_check_fs(ce, st);
210+
}
206211

207212
return changed;
208213
}
@@ -954,9 +959,7 @@ int write_cache(int newfd, struct cache_entry **cache, int entries)
954959
{
955960
SHA_CTX c;
956961
struct cache_header hdr;
957-
int i, removed, recent;
958-
struct stat st;
959-
time_t now;
962+
int i, removed;
960963

961964
for (i = removed = 0; i < entries; i++)
962965
if (!cache[i]->ce_mode)
@@ -994,57 +997,5 @@ int write_cache(int newfd, struct cache_entry **cache, int entries)
994997
return -1;
995998
}
996999
}
997-
998-
/*
999-
* To prevent later ce_match_stat() from always falling into
1000-
* check_fs(), if we have too many entries that can trigger
1001-
* racily clean check, we are better off delaying the return.
1002-
* We arbitrarily say if more than 20 paths or 25% of total
1003-
* paths are very new, we delay the return until the index
1004-
* file gets a new timestamp.
1005-
*
1006-
* NOTE! NOTE! NOTE!
1007-
*
1008-
* This assumes that nobody is touching the working tree while
1009-
* we are updating the index.
1010-
*/
1011-
1012-
/* Make sure that the new index file has st_mtime
1013-
* that is current enough -- ce_write() batches the data
1014-
* so it might not have written anything yet.
1015-
*/
1016-
ce_write_flush(&c, newfd);
1017-
1018-
now = fstat(newfd, &st) ? 0 : st.st_mtime;
1019-
if (now) {
1020-
recent = 0;
1021-
for (i = 0; i < entries; i++) {
1022-
struct cache_entry *ce = cache[i];
1023-
time_t entry_time = (time_t) ntohl(ce->ce_mtime.sec);
1024-
if (!ce->ce_mode)
1025-
continue;
1026-
if (now && now <= entry_time)
1027-
recent++;
1028-
}
1029-
if (20 < recent && entries <= recent * 4) {
1030-
#if 0
1031-
fprintf(stderr, "entries %d\n", entries);
1032-
fprintf(stderr, "recent %d\n", recent);
1033-
fprintf(stderr, "now %lu\n", now);
1034-
#endif
1035-
while (!fstat(newfd, &st) && st.st_mtime <= now) {
1036-
struct timespec rq, rm;
1037-
off_t where = lseek(newfd, 0, SEEK_CUR);
1038-
rq.tv_sec = 0;
1039-
rq.tv_nsec = 250000000;
1040-
nanosleep(&rq, &rm);
1041-
if ((where == (off_t) -1) ||
1042-
(write(newfd, "", 1) != 1) ||
1043-
(lseek(newfd, -1, SEEK_CUR) != where) ||
1044-
ftruncate(newfd, where))
1045-
break;
1046-
}
1047-
}
1048-
}
10491000
return ce_flush(&c, newfd);
10501001
}

0 commit comments

Comments
 (0)