Re: [PATCH v7 04/12] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

Ben Peart [off-list ref] writes:

This field being zero has more significance than just "we haven't
got any update yet", right?  The way I am reading the code is that
setting it 0 is a way to signal that fsmon has been inactivated.  It
also made me wonder if add_fsmonitor() that silently returns without
doing anything when this field is already non-zero is a bug (in
other words, I couldn't tell what the right answer would be to a
question "shouldn't the caller be avoiding duplicate calls?").

This part was a pleasant read.

Thanks.

On 9/20/2017 2:23 AM, Junio C Hamano wrote:

Ben Peart [off-list ref] writes:

quoted

@@ -344,6 +346,7 @@ struct index_state {
  	struct hashmap dir_hash;
  	unsigned char sha1[20];
  	struct untracked_cache *untracked;
+	uint64_t fsmonitor_last_update;

This field being zero has more significance than just "we haven't
got any update yet", right?  The way I am reading the code is that
setting it 0 is a way to signal that fsmon has been inactivated.  It
also made me wonder if add_fsmonitor() that silently returns without
doing anything when this field is already non-zero is a bug (in
other words, I couldn't tell what the right answer would be to a
question "shouldn't the caller be avoiding duplicate calls?").

Correct again.  For better (and sometimes for worse) I followed the 
pattern set by the untracked cache.  If you compare them, you will 
notice striking similarities. :)

quoted

diff --git a/fsmonitor.c b/fsmonitor.c
new file mode 100644
index 0000000000..b8b2d88fe1
--- /dev/null
+++ b/fsmonitor.c

...

This part was a pleasant read.

quoted

Thanks.

Thank you for your careful review.  I appreciate having another set of 
eyes taking a close look especially as I see this as a big first step 
towards making many git operations O(# changed files) instead of O(# 
size of working directory). Seeing status times drop from 1m22s to 1.45s 
is a huge perf win - but only if it is correct!