Re: [PATCH 1/2] fsmonitor: stop inline'ing mark_fsmonitor_valid / _invalid

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 21, 2020, 20:55 UTC
Message-ID: <20201021205537.GB1270359@nand.local>
In-Reply-To: <049989652cefb90304e711dbfe354b55a5a71f41.1603303474.git.gitgitgadget@gmail.com>

On Wed, Oct 21, 2020 at 06:04:33PM +0000, Alex Vandiver via GitGitGadget wrote:

Show 8 quoted lines

> From: Alex Vandiver <alexmv@dropbox.com>
>
> These were inline'd when they were first introduced, presumably as an
> optimization for cases when they were called in tight loops.  This
> complicates using these functions, as untracked_cache_invalidate_path
> is defined in dir.h.
>
> Leave the inline'ing up to the compiler's decision, for ease of use.

Letting the compiler inline these is fine, but...

Show 35 quoted lines

> diff --git a/fsmonitor.h b/fsmonitor.h
> index 739318ab6d..6249020692 100644
> --- a/fsmonitor.h
> +++ b/fsmonitor.h
> @@ -49,14 +49,7 @@ void refresh_fsmonitor(struct index_state *istate);
>   * called any time the cache entry has been updated to reflect the
>   * current state of the file on disk.
>   */
> -static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache_entry *ce)
> -{
> -	if (core_fsmonitor && !(ce->ce_flags & CE_FSMONITOR_VALID)) {
> -		istate->cache_changed = 1;
> -		ce->ce_flags |= CE_FSMONITOR_VALID;
> -		trace_printf_key(&trace_fsmonitor, "mark_fsmonitor_clean '%s'", ce->name);
> -	}
> -}
> +extern void mark_fsmonitor_valid(struct index_state *istate, struct cache_entry *ce);
>
>  /*
>   * Clear the given cache entry's CE_FSMONITOR_VALID bit and invalidate
> @@ -65,13 +58,6 @@ static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache
>   * trigger an lstat() or invalidate the untracked cache for the
>   * corresponding directory
>   */
> -static inline void mark_fsmonitor_invalid(struct index_state *istate, struct cache_entry *ce)
> -{
> -	if (core_fsmonitor) {
> -		ce->ce_flags &= ~CE_FSMONITOR_VALID;
> -		untracked_cache_invalidate_path(istate, ce->name, 1);
> -		trace_printf_key(&trace_fsmonitor, "mark_fsmonitor_invalid '%s'", ce->name);
> -	}
> -}
> +extern void mark_fsmonitor_invalid(struct index_state *istate, struct cache_entry *ce);
>
>  #endif

Any reason that these need to be externed explicitly? Note that these functions are already externed by default since you haven't said otherwise (and for no other reason than this'd be the only explicitly externed function in fsmonitor.h).

Thanks, Taylor