library: search processes by tags, not just name#2095
Conversation
| .interfaces<Process::ProcessFactoryList>() | ||
| .get(node.key)) | ||
| { | ||
| for(const QString& tag : f->descriptor(node.customData).tags) |
There was a problem hiding this comment.
This will be super bad for performance on systems with many presets :/ as f->descriptor(node.customData) is a potentially ultra-slow operation that I did my best to make sure is not called during searching operation (which is already too slow as-is)
There was a problem hiding this comment.
Thanks for checking and for taking care of keeping score efficiently.
New proposal: never touch descriptor() during search. Instead, precompute the tags once, when the list node is created, and store them on the node.
- Add a QStringList tag to Library::ProcessData.
- Populate it at node-creation time, which already happens off the search path:
- ProcessesItemModel::rescan() / on_newPlugin() for factory-backed processes — call descriptor({}).tags there, once per factory. (Or add a cheap tags() virtual on the factory returning the static Metadata<Tags_k>, to avoid descriptor() even here.)
- The async scanners (LV2/CLAP/VST/VST3/…) already parse the plugin metadata while building their nodes (LV2/Library.hpp reads the class label and plugin name in the same loop), so they can fill tags right there at no extra cost.
- filterAcceptsRowItself becomes a pure in-memory match: node.prettyName.contains(pattern) || any(node.tags, contains(pattern)). No factory lookup, no descriptor(), nothing allocated per keystroke.
Claude's assessment: This stays correct across library updates: the user library is a separate repo that can change on disk at any time, but node creation is already driven by RecursiveWatch, so whenever a file/plugin is (re)added, the tags are recomputed then. Same for custom processes — factory ones flow through on_newPlugin, file/preset ones through the scanners.
Claude checked, and no LibraryInterface scanner currently derives tags from descriptor(customData) (the only per-instance descriptor() calls are in the info panel / preset detail / rendering code, not in list building), so moving tags to scan time shouldn't lose anything.
One open question: for preset/file nodes, would you rather they inherit the parent process's tags, or only carry tags the scanner extracts from the file itself? Where a scanner sets none, node.tags is empty and name search behaves as today.
Does this direction look right before I rework the PR?
There was a problem hiding this comment.
There's two aspects:
-
Scanning the tags. The long-term goal for me is what was started with the GSoC : github.com/ossia/medialode ; the library should persist through a separate process (as 1. there are a LOT of crashes possible due to library scanning of various kinds of media, especially as soon as we want to scan more advanced tags for audio / video files etc. and 2. it takes time).
This way, tags would be saved in a SQLite, json or whatever database ; querying them wouldn't mean reparsing e.g. the faust / isf / ... (which since we have a few thousand shaders is actually a very non-negligible JSON parsing time on startup especially on windows), the built-in vst, lv2, jsfx etc. presets, and a fair amount of other things.
Don't add virtuals, the goal of the "descriptor" struct was actually to reduce the amount of virtuals which were also having a measurable performance impact at the scale of a few tens-of-thousands files in the library (my test machine has ~100k), + making things more painful to implement. -
Making search fast when everything is loaded. For this the simplest with QAbstractItemModel that we're using is adding a cached "tags" or "search string" to the model which would contain the tags.
I would favour adding something like:
struct ProcessData : Process::ProcessData
{
QIcon icon;
QString searchString;
};
on which we could feed name, tags, and any other information useful for search, for instance separated with commas - could be something like process name|file name|object description|tag 1|tag 2|tag 3|... (pipe chosen as separator as it cannot occur in filenames anywhere).
We could then fuzzy- (with rapidfuzz-cpp, part of the source tree) or exact-match it in
bool ProcessFilterProxy::filterAcceptsRowItself(
int srcRow, const QModelIndex& srcParent) const
{
QModelIndex index = sourceModel()->index(srcRow, 0, srcParent);
auto model = static_cast<ProcessesItemModel*>(sourceModel());
auto& node = model->nodeFromModelIndex(index);
return node.prettyName.contains(m_textPattern, Qt::CaseInsensitive);
}
This would also allow us to maybe remove the "case insensitive" match which also has a perf cost, by enforcing the search string to be lower-case (unless we want to give the option for case-sensitive search)
There was a problem hiding this comment.
A stop-gap solution would be to perform a two-level scan:
- The current scan which needs to be fast to scan by name, so that you don't have to wait 1-2 minute (average time to scan the full library with calls to descriptor() on a slow windows laptop) before the software gets useable ; we want all the process and main preset names to be on the library in a couple second tops.
- A second, deeper async scan running in a separate thread whose goal would be to compute the full search data, packetized like we do for RecursiveWatch (we don't want to clobber the main loop with tens of thousands of messages ; ~100 scan results per packet is a good sweet-spot to not make the UI lag and leave time to the main thread for processing UI events)
There was a problem hiding this comment.
(Replying on Edu's behalf — I'm Claude, his AI assistant; he'll follow up directly if I got anything wrong.)
Implemented the two-level scan you described — the branch is reworked and force-pushed (rebased on master):
- Fast scan untouched: names appear immediately, nothing changed on that path.
- Second, deeper scan on a worker thread: it computes a lower-case
|-separatedsearchString(name, description, tags) per node — this is wheredescriptor()gets called now. Results are delivered to the GUI thread in packets of 100, as you suggested.
Library::ProcessData now carries the searchString member per your snippet, and ProcessFilterProxy::filterAcceptsRowItself matches prettyName + searchString purely in-memory — no factory lookup, no descriptor(), nothing allocated on the search path.
A few implementation notes:
- Some scanners (LV2, the RecursiveWatch commit actions) add nodes without
begin/endInsertRows, so there's no reliable "node added" signal to hook. Instead a 3s timer polls for unindexed nodes; once everything is indexed the sweep is a cheap in-memory tree walk that collects nothing. If you'd rather have an explicit notification point (e.g. inaddToLibrary) I can rework that. - Node pointers handed to the worker are guarded by a generation counter bumped on
modelAboutToBeReset/rowsAboutToBeRemoved; stale packets are dropped and the nodes get reindexed on the next tick.TreeNodechildren arestd::list, so insertions don't invalidate the pointers. - Factories are resolved on the GUI thread before dispatch, so the worker never touches the interface list.
- Kept exact
contains()matching for now — happy to switch the proxy to rapidfuzz-cpp in a follow-up if you want fuzzy matching as part of this.
Tested on a ~24k-node library (Debug build): all nodes indexed shortly after startup in packets of 100, ISF shader JSON parsing happens entirely off the GUI thread, and once indexing is done the periodic sweep is a no-op.
The process library filter only matched against the displayed name. Per review, descriptor() must never be called on the search path: it can be extremely slow (it may parse the file behind the node, e.g. ISF/Faust) and search is already too slow as-is. Instead, implement the two-level scan suggested in the review: 1. The existing fast scan stays untouched: names appear immediately. 2. A second, deeper scan runs on a worker thread and computes a lower-case '|'-separated searchString (name, description, tags) per node by calling descriptor() there. Results are delivered to the GUI thread in packets of 100, like RecursiveWatch does, to avoid clobbering the main loop. Since some scanners (LV2, RecursiveWatch commits...) add nodes without begin/endInsertRows, there is no reliable "node added" signal: a 3s timer polls for unindexed nodes; the sweep is a cheap in-memory tree walk once everything is indexed. Node pointers handed to the worker are guarded by a generation counter bumped on model reset/row removal. The filter then matches prettyName and searchString purely in-memory. Fixes #1910 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sn5p9zXz8ZvWcC14x8hiWg
567caa1 to
dca2ffd
Compare
The process library filter only matched against the displayed name. Now, when the name doesn't match, also check the process tags by fetching the Descriptor via the factory list.
The factory lookup / descriptor() call only runs after the cheap name check fails, so filtering large libraries stays reasonably fast and an empty pattern never touches it.
Fixes #1910