On 2/23/24 12:36 PM, Junio C Hamano wrote:
"Jeff Hostetler via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
From: Jeff Hostetler <jeffhostetler@xxxxxxxxxx>
Move the call to invalidate the untracked cache for the FSEvent
pathname into the two helper functions.
In a later commit in this series, we will call these helpers
from other contexts and it safer to include the UC invalidation
in the helper than to remember to also add it to each helper
call-site.
Signed-off-by: Jeff Hostetler <jeffhostetler@xxxxxxxxxx>
---
fsmonitor.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
Thanks. The steps in this iteration makes this move much less
confusing to me than in the previous one. We used to call one of
"handle path with/without trailing slash" functions and then called
the invalidation. Now the invalidation happens in these "handle path"
functions.
The unexplained change in behaviour is that we used to do the rest
of "handle path" and invalidation was done at the end. Now we do it
upfront. I think the "rest" works solely based on what is in the
main in-core index array (i.e. the_index.cache[] aka active_cache[])
and affects only what is in the in-core index array, while
untracked_cache_invalidate*() works solely based on what is in the
untracked cache extension (i.e. the_index.untracked) and affects
only what is in there, so the order of these two does not matter.
Am I correct?
Or does it affect correctness or performance or whatever in any way?
IOW, is there a reason why it is better to do the invalidation first
and then doing the "rest" after (hence this patch flips the order of
two to _improve_ something)?
Thanks.
The ce_flags invalidation and the untracked-cache invalidation are
independent (as far as I could tell) and it doesn't matter which
order we do them. Moving the UC to the start of the function was
an attempt to avoid the usual "goto the bottom" or the need to guard
against early "return" statements that were present in some of the
original code (or my various refactorings). Moving it to the top
just let me get it out of the way and not have to contrive things.
I'll update the commit message.
Thanks
Jeff