Re: [PATCH 12/23] fsmonitor--daemon: create token-based changed path cache

On 4/26/21 4:22 PM, Derrick Stolee wrote:
...

quoted

+
+void fsmonitor_batch__add_path(struct fsmonitor_batch *batch,
+			       const char *path)
+{
+	const char *interned_path = strintern(path);

This use of interned paths is interesting, although I become
concerned for the amount of memory we are consuming over the
lifetime of the process. This could be considered as a target
for future improvements, perhaps with an LRU cache or something
similar.

Interning gives us a fixed pointer for any given path.  This
gives us a way to de-dup paths using just pointers rather than
string compares.

Yes, we will accumulate paths in that dictionary, but the set of
paths present in the typical working directory are usually pretty
fixed.

We only generate these for modified paths.  Users don't typically
create/modify/delete that many paths in their source trees during
normal development.

Compilers may generate lots of trash files in their worktree, but
those names are usually repeated (with each "make").  So we might
acculuate a lot of paths for a repo, it should become stable.
However, if they use temp files in the tree, it might invalidate
this statement.

WRT LRUs, that gets us into threading and lock contention problem
and ref-counting.  I have it designed such that parallel threads
read and send the current queue to the client without a lock.  They
only need a quick lock to get the current head pointer; the rest
is done lock free.  Also, purging from the end of the LRU would
put is in contention with the FS listener thread that is adding
new paths to the LRU.

So, yeah, maybe this is something to keep an eye on -- especially
in the monorepo case, but I don't think we need to address it now.

Thanks,
Jeff