refactor: library scanning - #1495
Conversation
CyanVoxel
left a comment
There was a problem hiding this comment.
Oddly this significantly slows down scanning performance for me, on my NAS library at least... scanning a local library does seem to be much faster. However this also removes the cache of previously scanned files, so subsequent scans are no longer lightning fast and take up the full scan time again.
I don't recall #1242 + this interactive subprocess comment creating a slowdown, so it may be worth comparing what #1242 is doing differently (without creating the same normalization issue there). Also, while I haven't tested this in compiled builds, I'd lean towards using the silent subprocess method we have to avoid spawning CMD windows in builds (I'm not sure if ripgrep will do that, but other subprocesses like ffmpeg do normally).
ignore_to_glob()produces a lot of redundant, recursive patterns that slow down the internal scanner so much that it's quite literally unusable on my primary library
I agree that it produces a lot of redundant patterns. It was the best I could do at the time in order for it to match the functionality of .gitignore, and am open to improvements to it.
Is there a reason refresh_dir() is the one responsible for enforcing this limit? At least to me personally, it'd make more sense to place the responsibility on the UI for limiting the rate of UI updates. Especially if another, rapidly-updating data source were connected to a progress UI.
It may be a good idea to create a general utility method for enforcing an update limit (say, it accepts an iterator, and returns an iterator limited to a certain update limit).
If there's a good efficient way to restrict the rapid pinging from the backend side on the UI side, I'm open to it
It could be worth having ripgrep use null separators, rather than new lines, to prevent issues with certain file names.
Sounds good if straightforward
Summary
refresh_dir(), which consumes the files produced by which ever scanner is used.scan_for_files(). This way, any function that needs to scan for files can just blindly callscan_for_files()without needing to determine which scanner should be used, and it will receive an iterator of files.wcmatchscanner to simply "internal". This better reflects its purpose, and helps if the implementation of the internal scanner ever changes down the line.scan_for_files()) static, as they don't rely on any attributes ofRefreshTracker.Notes
ignore_to_glob()produces a lot of redundant, recursive patterns that slow down the internal scanner so much that it's quite literally unusable on my primary library (~65k files, with a large ignore file). I was never able to get it to produce any output on my primary library. It may be worth looking into improving the translation from gitignore-style to glob, or replacing the internal scanner implementation altogether.refresh_dir()emit more detailed progress, rather than just anint.refresh_dir()is set to only emit output every 1/30th of a second.refresh_dir()is the one responsible for enforcing this limit? At least to me personally, it'd make more sense to place the responsibility on the UI for limiting the rate of UI updates. Especially if another, rapidly-updating data source were connected to a progress UI.Tasks Completed