Skip to content

Commit 700696b

Browse files
gitstergitster
committed
Merge branch 'jh/fsmonitor-prework'
Preliminary changes to fsmonitor integration. * jh/fsmonitor-prework: fsmonitor: refactor initialization of fsmonitor_last_update token fsmonitor: allow all entries for a folder to be invalidated fsmonitor: log FSMN token when reading and writing the index fsmonitor: log invocation of FSMonitor hook to trace2 read-cache: log the number of scanned files to trace2 read-cache: log the number of lstat calls to trace2 preload-index: log the number of lstat calls to trace2 p7519: add trace logging during perf test p7519: move watchman cleanup earlier in the test p7519: fix watchman watch-list test on Windows p7519: do not rely on "xargs -d" in test
2 parents 9091737 + fcd19b0 commit 700696b

File tree

7 files changed

+196
-26
lines changed

7 files changed

+196
-26
lines changed

‎fsmonitor.c‎

Lines changed: 96 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,11 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data,
9090
if (!istate->split_index)
9191
assert_index_minimum(istate, istate->fsmonitor_dirty->bit_size);
9292

93-
trace_printf_key(&trace_fsmonitor, "read fsmonitor extension successful");
93+
trace2_data_string("index", NULL, "extension/fsmn/read/token",
94+
istate->fsmonitor_last_update);
95+
trace_printf_key(&trace_fsmonitor,
96+
"read fsmonitor extension successful '%s'",
97+
istate->fsmonitor_last_update);
9498
return 0;
9599
}
96100

@@ -134,7 +138,11 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
134138
put_be32(&ewah_size, sb->len - ewah_start);
135139
memcpy(sb->buf + fixup, &ewah_size, sizeof(uint32_t));
136140

137-
trace_printf_key(&trace_fsmonitor, "write fsmonitor extension successful");
141+
trace2_data_string("index", NULL, "extension/fsmn/write/token",
142+
istate->fsmonitor_last_update);
143+
trace_printf_key(&trace_fsmonitor,
144+
"write fsmonitor extension successful '%s'",
145+
istate->fsmonitor_last_update);
138146
}
139147

140148
/*
@@ -143,6 +151,7 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
143151
static int query_fsmonitor(int version, const char *last_update, struct strbuf *query_result)
144152
{
145153
struct child_process cp = CHILD_PROCESS_INIT;
154+
int result;
146155

147156
if (!core_fsmonitor)
148157
return -1;
@@ -153,16 +162,63 @@ static int query_fsmonitor(int version, const char *last_update, struct strbuf *
153162
cp.use_shell = 1;
154163
cp.dir = get_git_work_tree();
155164

156-
return capture_command(&cp, query_result, 1024);
165+
trace2_region_enter("fsm_hook", "query", NULL);
166+
167+
result = capture_command(&cp, query_result, 1024);
168+
169+
if (result)
170+
trace2_data_intmax("fsm_hook", NULL, "query/failed", result);
171+
else {
172+
trace2_data_intmax("fsm_hook", NULL, "query/response-length",
173+
query_result->len);
174+
175+
if (fsmonitor_is_trivial_response(query_result))
176+
trace2_data_intmax("fsm_hook", NULL,
177+
"query/trivial-response", 1);
178+
}
179+
180+
trace2_region_leave("fsm_hook", "query", NULL);
181+
182+
return result;
157183
}
158184

159-
static void fsmonitor_refresh_callback(struct index_state *istate, const char *name)
185+
int fsmonitor_is_trivial_response(const struct strbuf *query_result)
160186
{
161-
int pos = index_name_pos(istate, name, strlen(name));
187+
static char trivial_response[3] = { '\0', '/', '\0' };
188+
int is_trivial = !memcmp(trivial_response,
189+
&query_result->buf[query_result->len - 3], 3);
190+
191+
return is_trivial;
192+
}
193+
194+
static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
195+
{
196+
int i, len = strlen(name);
197+
if (name[len - 1] == '/') {
198+
199+
/*
200+
* TODO We should binary search to find the first path with
201+
* TODO this directory prefix. Then linearly update entries
202+
* TODO while the prefix matches. Taking care to search without
203+
* TODO the trailing slash -- because '/' sorts after a few
204+
* TODO interesting special chars, like '.' and ' '.
205+
*/
206+
207+
/* Mark all entries for the folder invalid */
208+
for (i = 0; i < istate->cache_nr; i++) {
209+
if (istate->cache[i]->ce_flags & CE_FSMONITOR_VALID &&
210+
starts_with(istate->cache[i]->name, name))
211+
istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
212+
}
213+
/* Need to remove the / from the path for the untracked cache */
214+
name[len - 1] = '\0';
215+
} else {
216+
int pos = index_name_pos(istate, name, strlen(name));
162217

163-
if (pos >= 0) {
164-
struct cache_entry *ce = istate->cache[pos];
165-
ce->ce_flags &= ~CE_FSMONITOR_VALID;
218+
if (pos >= 0) {
219+
struct cache_entry *ce = istate->cache[pos];
220+
ce->ce_flags &= ~CE_FSMONITOR_VALID;
221+
}
166222
}
167223

168224
/*
@@ -288,16 +344,45 @@ void refresh_fsmonitor(struct index_state *istate)
288344
istate->fsmonitor_last_update = strbuf_detach(&last_update_token, NULL);
289345
}
290346

347+
/*
348+
* The caller wants to turn on FSMonitor. And when the caller writes
349+
* the index to disk, a FSMonitor extension should be included. This
350+
* requires that `istate->fsmonitor_last_update` not be NULL. But we
351+
* have not actually talked to a FSMonitor process yet, so we don't
352+
* have an initial value for this field.
353+
*
354+
* For a protocol V1 FSMonitor process, this field is a formatted
355+
* "nanoseconds since epoch" field. However, for a protocol V2
356+
* FSMonitor process, this field is an opaque token.
357+
*
358+
* Historically, `add_fsmonitor()` has initialized this field to the
359+
* current time for protocol V1 processes. There are lots of race
360+
* conditions here, but that code has shipped...
361+
*
362+
* The only true solution is to use a V2 FSMonitor and get a current
363+
* or default token value (that it understands), but we cannot do that
364+
* until we have actually talked to an instance of the FSMonitor process
365+
* (but the protocol requires that we send a token first...).
366+
*
367+
* For simplicity, just initialize like we have a V1 process and require
368+
* that V2 processes adapt.
369+
*/
370+
static void initialize_fsmonitor_last_update(struct index_state *istate)
371+
{
372+
struct strbuf last_update = STRBUF_INIT;
373+
374+
strbuf_addf(&last_update, "%"PRIu64"", getnanotime());
375+
istate->fsmonitor_last_update = strbuf_detach(&last_update, NULL);
376+
}
377+
291378
void add_fsmonitor(struct index_state *istate)
292379
{
293380
unsigned int i;
294-
struct strbuf last_update = STRBUF_INIT;
295381

296382
if (!istate->fsmonitor_last_update) {
297383
trace_printf_key(&trace_fsmonitor, "add fsmonitor");
298384
istate->cache_changed |= FSMONITOR_CHANGED;
299-
strbuf_addf(&last_update, "%"PRIu64"", getnanotime());
300-
istate->fsmonitor_last_update = strbuf_detach(&last_update, NULL);
385+
initialize_fsmonitor_last_update(istate);
301386

302387
/* reset the fsmonitor state */
303388
for (i = 0; i < istate->cache_nr; i++)

‎fsmonitor.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ void tweak_fsmonitor(struct index_state *istate);
4444
*/
4545
void refresh_fsmonitor(struct index_state *istate);
4646

47+
/*
48+
* Does the received result contain the "trivial" response?
49+
*/
50+
int fsmonitor_is_trivial_response(const struct strbuf *query_result);
51+
4752
/*
4853
* Set the given cache entries CE_FSMONITOR_VALID bit. This should be
4954
* called any time the cache entry has been updated to reflect the

‎preload-index.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ struct thread_data {
3131
struct pathspec pathspec;
3232
struct progress_data *progress;
3333
int offset, nr;
34+
int t2_nr_lstat;
3435
};
3536

3637
static void *preload_thread(void *_data)
@@ -73,6 +74,7 @@ static void *preload_thread(void *_data)
7374
continue;
7475
if (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce)))
7576
continue;
77+
p->t2_nr_lstat++;
7678
if (lstat(ce->name, &st))
7779
continue;
7880
if (ie_match_stat(index, ce, &st, CE_MATCH_RACY_IS_DIRTY|CE_MATCH_IGNORE_FSMONITOR))
@@ -98,6 +100,7 @@ void preload_index(struct index_state *index,
98100
int threads, i, work, offset;
99101
struct thread_data data[MAX_PARALLEL];
100102
struct progress_data pd;
103+
int t2_sum_lstat = 0;
101104

102105
if (!HAVE_THREADS || !core_preload_index)
103106
return;
@@ -107,6 +110,9 @@ void preload_index(struct index_state *index,
107110
threads = 2;
108111
if (threads < 2)
109112
return;
113+
114+
trace2_region_enter("index", "preload", NULL);
115+
110116
trace_performance_enter();
111117
if (threads > MAX_PARALLEL)
112118
threads = MAX_PARALLEL;
@@ -141,10 +147,14 @@ void preload_index(struct index_state *index,
141147
struct thread_data *p = data+i;
142148
if (pthread_join(p->pthread, NULL))
143149
die("unable to join threaded lstat");
150+
t2_sum_lstat += p->t2_nr_lstat;
144151
}
145152
stop_progress(&pd.progress);
146153

147154
trace_performance_leave("preload index");
155+
156+
trace2_data_intmax("index", NULL, "preload/sum_lstat", t2_sum_lstat);
157+
trace2_region_leave("index", "preload", NULL);
148158
}
149159

150160
int repo_read_index_preload(struct repository *repo,

‎read-cache.c‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,7 +1364,9 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
13641364
static struct cache_entry *refresh_cache_ent(struct index_state *istate,
13651365
struct cache_entry *ce,
13661366
unsigned int options, int *err,
1367-
int *changed_ret)
1367+
int *changed_ret,
1368+
int *t2_did_lstat,
1369+
int *t2_did_scan)
13681370
{
13691371
struct stat st;
13701372
struct cache_entry *updated;
@@ -1406,6 +1408,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
14061408
return NULL;
14071409
}
14081410

1411+
if (t2_did_lstat)
1412+
*t2_did_lstat = 1;
14091413
if (lstat(ce->name, &st) < 0) {
14101414
if (ignore_missing && errno == ENOENT)
14111415
return ce;
@@ -1442,6 +1446,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
14421446
}
14431447
}
14441448

1449+
if (t2_did_scan)
1450+
*t2_did_scan = 1;
14451451
if (ie_modified(istate, ce, &st, options)) {
14461452
if (err)
14471453
*err = EINVAL;
@@ -1519,6 +1525,8 @@ int refresh_index(struct index_state *istate, unsigned int flags,
15191525
const char *added_fmt;
15201526
const char *unmerged_fmt;
15211527
struct progress *progress = NULL;
1528+
int t2_sum_lstat = 0;
1529+
int t2_sum_scan = 0;
15221530

15231531
if (flags & REFRESH_PROGRESS && isatty(2))
15241532
progress = start_delayed_progress(_("Refresh index"),
@@ -1536,11 +1544,14 @@ int refresh_index(struct index_state *istate, unsigned int flags,
15361544
* we only have to do the special cases that are left.
15371545
*/
15381546
preload_index(istate, pathspec, 0);
1547+
trace2_region_enter("index", "refresh", NULL);
15391548
for (i = 0; i < istate->cache_nr; i++) {
15401549
struct cache_entry *ce, *new_entry;
15411550
int cache_errno = 0;
15421551
int changed = 0;
15431552
int filtered = 0;
1553+
int t2_did_lstat = 0;
1554+
int t2_did_scan = 0;
15441555

15451556
ce = istate->cache[i];
15461557
if (ignore_submodules && S_ISGITLINK(ce->ce_mode))
@@ -1566,7 +1577,11 @@ int refresh_index(struct index_state *istate, unsigned int flags,
15661577
if (filtered)
15671578
continue;
15681579

1569-
new_entry = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);
1580+
new_entry = refresh_cache_ent(istate, ce, options,
1581+
&cache_errno, &changed,
1582+
&t2_did_lstat, &t2_did_scan);
1583+
t2_sum_lstat += t2_did_lstat;
1584+
t2_sum_scan += t2_did_scan;
15701585
if (new_entry == ce)
15711586
continue;
15721587
if (progress)
@@ -1602,6 +1617,9 @@ int refresh_index(struct index_state *istate, unsigned int flags,
16021617

16031618
replace_index_entry(istate, i, new_entry);
16041619
}
1620+
trace2_data_intmax("index", NULL, "refresh/sum_lstat", t2_sum_lstat);
1621+
trace2_data_intmax("index", NULL, "refresh/sum_scan", t2_sum_scan);
1622+
trace2_region_leave("index", "refresh", NULL);
16051623
if (progress) {
16061624
display_progress(progress, istate->cache_nr);
16071625
stop_progress(&progress);
@@ -1614,7 +1632,7 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate,
16141632
struct cache_entry *ce,
16151633
unsigned int options)
16161634
{
1617-
return refresh_cache_ent(istate, ce, options, NULL, NULL);
1635+
return refresh_cache_ent(istate, ce, options, NULL, NULL, NULL, NULL);
16181636
}
16191637

16201638

‎t/perf/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
/build/
22
/test-results/
3+
/test-trace/
34
/trash directory*/

‎t/perf/Makefile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ perf: pre-clean
77
./run
88

99
pre-clean:
10-
rm -rf test-results
10+
rm -rf test-results test-trace
1111

1212
clean:
13-
rm -rf build "trash directory".* test-results
13+
rm -rf build "trash directory".* test-results test-trace
1414

1515
test-lint:
1616
$(MAKE) -C .. test-lint

0 commit comments

Comments
 (0)