On 4/26/21 4:22 PM, Derrick Stolee wrote: ...
+ +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
