Skip to content

library: search processes by tags, not just name - #2095

Open
edumeneses wants to merge 2 commits into
masterfrom
1910-process-search-tags
Open

edumeneses wants to merge 2 commits into
masterfrom
1910-process-search-tags

Conversation

@edumeneses

Copy link
Copy Markdown
Contributor

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

@edumeneses
edumeneses requested a review from jcelerier June 22, 2026 21:15
@edumeneses edumeneses self-assigned this Jun 23, 2026
.interfaces<Process::ProcessFactoryList>()
.get(node.key))
{
for(const QString& tag : f->descriptor(node.customData).tags)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@jcelerier jcelerier Jul 13, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's two aspects:

  1. 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.

  2. 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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A stop-gap solution would be to perform a two-level scan:

  1. 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.
  2. 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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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):

  1. Fast scan untouched: names appear immediately, nothing changed on that path.
  2. Second, deeper scan on a worker thread: it computes a lower-case |-separated searchString (name, description, tags) per node — this is where descriptor() 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. in addToLibrary) 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. TreeNode children are std::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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging this, we'll prepare another patch that makes sue that all the library work always uses proper QAbstractItemModel features instead of dumb resetModel

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude prepare a PR for making sure Library scanners always use insert / remove rows, and not a full-blown model reset.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Replying on Edu's behalf — I'm Claude, his AI assistant; he'll follow up directly if I got anything wrong.)

Done — pushed to 1910-process-search-tags (commit 0d0581c02).

Converted the scanner mutation paths from full beginResetModel/endResetModel (or silent tree mutation) to proper row operations:

  • CLAP (Clap/Library.cpp): the pluginsChanged rebuild now removes the old subtree with beginRemoveRows and inserts the new one with beginInsertRows, mirroring the VST3 handler. The subtree is built into a detached node and moved in under a single insert.
  • LV2 (LV2/Library.hpp): the async scan result is built off to the side and appended under one beginInsertRows instead of emplacing into the live tree with no signals.
  • ProcessesItemModel::on_newPlugin: wraps its category/process inserts in begin/endInsertRows (added a small nodeToIndex() helper to locate a node's QModelIndex).
  • PresetItemModel::savePreset: inserts the new preset row with begin/endInsertRows instead of resetting the whole list.

Pattern throughout: build additions into a detached temp ProcessNode, then bulk-move them in under one beginInsertRows. TreeNode's move ctor repoints child parent pointers at every level, so the moves are safe. This keeps addToLibrary, VST/VST3, and the addons untouched.

Two things I deliberately left alone — let me know if you want them in this PR or a follow-up:

  • The startup ProcessesItemModel::rescan() full rebuild (the view is empty at that point, so the reset is harmless) and the search-proxy filter reset (not a scanner).
  • Remaining silent add paths: the Subcategories helper and the airwindows/deuterium addon scanners. Migrating those means threading the model reference through Subcategories, so I kept it out for now.

Built and compiled the four affected translation units locally (clean).

@edumeneses
edumeneses force-pushed the 1910-process-search-tags branch from 567caa1 to dca2ffd Compare July 14, 2026 13:25
edumeneses and others added 2 commits October 9, 2026 22:25
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 the library model is now the only code that mutates the tree and
always emits row signals (7ad40f1), the deep scan follows rowsInserted
and modelReset, coalesced by a 250 ms single-shot timer; rows arriving
while a scan runs trigger another one once it finishes. Node pointers
handed to the worker are guarded by a generation counter bumped on model
reset/row removal; a scan whose results were dropped that way runs again.
An indexed node always has a non-empty searchString ("|" when there is
nothing to add), so empty means "not indexed yet".

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
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lc7tYGHRn6TmaxMEfBTswW
The search index worker calls ProcessModelFactory::descriptor() for every
process node. The application destroys its plug-ins and interface lists
when the presenter goes, and the library panel, which owns the model, only
with the view after that, so a scan still running then called a destroyed
factory. Flipping a flag in the model's destructor came too late, and
did not wait for a call in progress either. test_integration_js_presets
crashed there in most runs, since indexing now starts a moment after the
library is loaded instead of on a 3 s poll.

score::CancellationToken is a small shared stop flag with a mutex: run(f)
calls f unless the token is cancelled, and cancel() waits for a call in
progress. Live tokens are cancelled together by cancelAll(), which
ApplicationComponentsData's destructor calls right after
RecursiveWatch::cancelAll(), before any plug-in is deleted. The model
cancels its token when it is destroyed too, and the worker runs every
descriptor() call and every post back to the model through it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lc7tYGHRn6TmaxMEfBTswW
@edumeneses
edumeneses force-pushed the 1910-process-search-tags branch from 0d0581c to 4a23aaf Compare October 11, 2026 03:04
@edumeneses

Copy link
Copy Markdown
Contributor Author

This is Edu's Claude assistant, posting on Edu's behalf.

Rebased onto master (4b2f7f45ef). There are two commits now:

  • Dropped library: use insert/remove rows in scanners instead of model reset. JM's 7ad40f1 (staged subtree publish, replaceChildren(), on_newPlugin insert signals) and b08bb02 (preset model insertions with exact signals) do all of it and more, so nothing from that commit was left to apply.
  • Kept library: search processes by tags, not just name, adapted to the new model:
    • searchString moves to Library::ProcessData in ProcessEntry.hpp, where that struct lives now.
    • No more 3 s poll. The poll was only there because some scanners added nodes without row signals. Since 7ad40f1 the model is the only code that mutates the tree, so the deep scan now follows rowsInserted and modelReset, coalesced by a 250 ms single-shot timer. Rows that arrive while a scan runs start another one when it finishes. A scan whose results were dropped by the generation counter (a reset or row removal in between) runs again.
    • The code comments are gone; the commit message carries the explanation.

The rest is as reviewed: descriptor() only runs on the worker, results come back in packets of 100, and the filter matches prettyName and searchString in memory.

New library: the search indexing stops before what it calls is destroyed. Found because test_integration_js_presets started crashing with this PR. The search worker calls descriptor() on the factories, but the application destroys its plug-ins and interfaces with the presenter, and the library panel that owns the model only goes with the view afterwards. So a scan still running at exit called a destroyed factory. The 3 s poll had mostly hidden this, since short runs never got to index.

The fix follows d2099a4. A small score::CancellationToken (in score/tools) is a stop flag plus a mutex: run(f) skips f once cancelled, and cancel() waits for a call in progress. ApplicationComponentsData's destructor cancels every live token right after RecursiveWatch::cancelAll(), and the model cancels its own when destroyed. Every descriptor() call and every post back to the model goes through it. There is a unit test, test_unit_cancellation_token.

Checked: Debug build (clang 19) of master + this + Edu's other open PRs + #2302. test_integration_js_presets crashed in 3 of 4 runs before the fix and passes 8 of 8 after. test_library_publish fails at ProcessLibraryPublishTest.cpp:380 (alpha.isValid()) with or without this PR.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage is 47.802% — 1910-process-search-tags into master. No base build found for master.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Aliases for process search

3 participants