fsmonitor: enhance existing comments, clarify trivial response handling

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff Hostetler committed Mar 25, 2022 at 18:02 UTC 974c1b398711b8782c01bbd3aec959a458296da8
1 file changed +41 -23
fsmonitor.c
+41 -23
@@ -168,29 +168,15 @@ static int query_fsmonitor(int version, const char *last_update, struct strbuf *
168
169 if (result)
170 trace2_data_intmax("fsm_hook", NULL, "query/failed", result);
171 - else {
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 -
175 trace2_region_leave("fsm_hook", "query", NULL);
176
177 return result;
178 }
179
185 -int fsmonitor_is_trivial_response(const struct strbuf *query_result)
186 -{
187 - static char trivial_response[3] = { '\0', '/', '\0' };
188 -
189 - return query_result->len >= 3 &&
190 - !memcmp(trivial_response,
191 - &query_result->buf[query_result->len - 3], 3);
192 -}
193 -
180 static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
181 {
182 int i, len = strlen(name);
@@ -238,6 +224,7 @@ void refresh_fsmonitor(struct index_state *istate)
224 struct strbuf last_update_token = STRBUF_INIT;
225 char *buf;
226 unsigned int i;
227 + int is_trivial = 0;
228
229 if (!core_fsmonitor || istate->fsmonitor_has_run_once)
230 return;
@@ -283,6 +270,7 @@ void refresh_fsmonitor(struct index_state *istate)
270 query_success = 0;
271 } else {
272 bol = last_update_token.len + 1;
273 + is_trivial = query_result.buf[bol] == '/';
274 }
275 } else if (hook_version < 0) {
276 hook_version = HOOK_INTERFACE_VERSION1;
@@ -294,16 +282,38 @@ void refresh_fsmonitor(struct index_state *istate)
282 if (hook_version == HOOK_INTERFACE_VERSION1) {
283 query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION1,
284 istate->fsmonitor_last_update, &query_result);
285 + if (query_success)
286 + is_trivial = query_result.buf[0] == '/';
287 }
288
289 + if (is_trivial)
290 + trace2_data_intmax("fsm_hook", NULL,
291 + "query/trivial-response", 1);
292 +
293 trace_performance_since(last_update, "fsmonitor process '%s'", core_fsmonitor);
294 trace_printf_key(&trace_fsmonitor, "fsmonitor process '%s' returned %s",
295 core_fsmonitor, query_success ? "success" : "failure");
296 }
297
304 - /* a fsmonitor process can return '/' to indicate all entries are invalid */
305 - if (query_success && query_result.buf[bol] != '/') {
306 - /* Mark all entries returned by the monitor as dirty */
298 + /*
299 + * The response from FSMonitor (excluding the header token) is
300 + * either:
301 + *
302 + * [a] a (possibly empty) list of NUL delimited relative
303 + * pathnames of changed paths. This list can contain
304 + * files and directories. Directories have a trailing
305 + * slash.
306 + *
307 + * [b] a single '/' to indicate the provider had no
308 + * information and that we should consider everything
309 + * invalid. We call this a trivial response.
310 + */
311 + if (query_success && !is_trivial) {
312 + /*
313 + * Mark all pathnames returned by the monitor as dirty.
314 + *
315 + * This updates both the cache-entries and the untracked-cache.
316 + */
317 buf = query_result.buf;
318 for (i = bol; i < query_result.len; i++) {
319 if (buf[i] != '\0')
@@ -318,11 +328,16 @@ void refresh_fsmonitor(struct index_state *istate)
328 if (istate->untracked)
329 istate->untracked->use_fsmonitor = 1;
330 } else {
321 -
322 - /* We only want to run the post index changed hook if we've actually changed entries, so keep track
323 - * if we actually changed entries or not */
331 + /*
332 + * We failed to get a response or received a trivial response,
333 + * so invalidate everything.
334 + *
335 + * We only want to run the post index changed hook if
336 + * we've actually changed entries, so keep track if we
337 + * actually changed entries or not.
338 + */
339 int is_cache_changed = 0;
325 - /* Mark all entries invalid */
340 +
341 for (i = 0; i < istate->cache_nr; i++) {
342 if (istate->cache[i]->ce_flags & CE_FSMONITOR_VALID) {
343 is_cache_changed = 1;
@@ -330,7 +345,10 @@ void refresh_fsmonitor(struct index_state *istate)
345 }
346 }
347
333 - /* If we're going to check every file, ensure we save the results */
348 + /*
349 + * If we're going to check every file, ensure we save
350 + * the results.
351 + */
352 if (is_cache_changed)
353 istate->cache_changed |= FSMONITOR_CHANGED;
354