> from Taylor > I'm still iffy on whether or not this series makes sense to apply > without the rest of the code that depends on it Sorry for confusion. I don't think we should assume there is more code coming related to this. I think this is intended to stand on its own. It's not a required dependency either. Rather, it's motivated by simplicity - remove the dir.h dependency from fsmonitor.h. - Keep implementation in fsmonitor.c and definitions in fsmonitor.h > From Junio > Those without fsmonitor would pay the call/return cost for no good > reason if core_fsmonitor is not set, and checking that on the caller > side may make a big difference. How big? That needs measurement. Noted! This is not called out or measured - it is simply assumed based on earlier conversation. I should be able to run the fsmonitor perf suite before/after this change and include the results in the commit message. > This is a tangent, but with or without inlining, I find it iffy to > see that untracked_cache_invalidate_path() is called only when > fsmonitor is in use. Does untracked_cache depend on fsmonitor for > its correct operation? Why is it OK not to invlidate when the > caller would tell fsmonitor that a path is invalid if fsmonitor were > in use? The call is a statement of fact that the path is no longer > valid, and that bit of information would be useful to the parts of > the system outside fsmonitor, no? Puzzled.... I did some source diving in an attempt to understand what's happening here. I believe that untracked_cache_invalidate_path() is called in dir.c whenever an entry is added or removed from a directory. This is an additional call when fsmonitor is enabled - because fsmonitor's whole purpose is to avoid the lstat on the other path. There is a nice explanation in the original commit message Commit 883e248b (fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files., 2017-09-22) --Nipunn
