Skip to content

Commit ab7f655

Browse files
committed
feat(search): caller-owned content-search drain pool in SearchService
SearchService now owns a pool of drain threads that loop vxcore_work_queue_process_next on the "vxcore.search" queue, powering vxcore's caller-helps-drain content search (vxcore no longer owns a search thread pool). Pool size is min(hardware_concurrency, 8), substituting 2 when the count is unknown. Threads spawn after the worker thread starts and join in the dtor BEFORE the mutex is deleted, while the vxcore context is still alive; idle threads block on the queue condvar (~0 CPU) since the queue is pre-created. - SearchCoreService::context() accessor for the drain loop. - tests/core/test_search_service_drain: results-match-baseline, idle-no-spin, teardown-during-active-search (watchdog), cancel-with-backlog. Green. - Docs: Search Threading Contract (root AGENTS.md) + SearchService drain pool (src/core/services/AGENTS.md) + vxcore Search backends note. - Bumps libs/vxcore to the caller-owned vxcore.search WorkQueue commit.
1 parent 650eb0e commit ab7f655

8 files changed

Lines changed: 450 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,16 @@ The legacy `Buffer` (1-second timer in `src/core/buffer/buffer.cpp`) bypasses vx
501501
502502
---
503503
504+
## Search Threading Contract
505+
506+
Content search in vxcore owns NO thread pool. `vxcore_search_content` / `vxcore_search_content_ex` enqueue ONE work item per file onto a dedicated `"vxcore.search"` `WorkQueue` that is pre-created at `vxcore_context_create`. The CALLER's threads drain that queue, and the initiating thread help-drains its own enqueued items (caller-helps-drain: it loops `ProcessNext(5ms)` until the batch is done). VNote's [`SearchService`](src/core/services/searchservice.h) owns the drain pool that loops `vxcore_work_queue_process_next(ctx, "vxcore.search", 100)`. Batches under 50 files skip the queue and run inline sequentially.
507+
508+
The initiating thread's self-drain is the correctness floor: a consumer that provides NO external drain threads still gets correct, single-threaded results, and extra drainers only add parallelism. Cancellation, `max_results`, and result ordering are preserved across both paths; an exception thrown mid-scan is caught and surfaces as `VXCORE_ERR_UNKNOWN`.
509+
510+
This mirrors the vxcore/VNote ownership split used by sync: vxcore emits per-file search work as facts, VNote owns the drain policy. No vxcore-owned threads remain, the former `BS::thread_pool` search pool having been removed.
511+
512+
---
513+
504514
## Code Style Guidelines
505515
506516
### Standards

src/core/services/AGENTS.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,15 @@ The keychain PAT for a notebook is tied to its lifecycle. To avoid orphan vault
115115
| S6 startup sweep | `src/core/services/syncservice.cpp:1280` (`onMainWindowAfterStart`) | App start, for each notebook where `!isSyncEnabled(id) && m_credentialsStore->hasCredentials(id)` (disk says disabled but keychain still holds a PAT). Backstop for previous-session crashes between the JSON-clear and keychain-delete steps. | Disk and keychain already agree (normal case). |
116116

117117
**Rule for new sync-related code paths**: any time you retire a notebook, roll back an enable, or transition to a state where the on-disk JSON no longer claims sync is enabled, route through one of the five sites above. Do not call `deleteCredentials` from controllers or widgets; the cleanup contract lives in `SyncService` (and the one historical exception in `NewNotebookController::bootstrapSync`, which is documented in `src/controllers/AGENTS.md`).
118+
119+
## SearchService drain pool
120+
121+
`SearchService` owns the pool of drain threads that empty vxcore's `"vxcore.search"` work queue (see the root [Search Threading Contract](../../../AGENTS.md#search-threading-contract)). vxcore owns no search threads; this pool is VNote's side of that contract.
122+
123+
**Size.** `min(std::thread::hardware_concurrency(), 8)`, substituting `2` only when `hardware_concurrency()` returns `0` (count unknown). This is a fallback for the unknown case, not a floor: a genuine single-core host gets `1` drain thread. Each thread loops `vxcore_work_queue_process_next(ctx, "vxcore.search", 100)`.
124+
125+
**Lifetime.** The drain threads are spawned in the `SearchService` constructor AFTER the worker thread has started, and torn down in the destructor by setting `m_stopDrain` and joining every drain thread BEFORE the queue mutex is deleted and while the vxcore context is still alive. Joining first prevents a drain thread from touching a half-destroyed queue or a freed context.
126+
127+
**Idle cost.** Because the `"vxcore.search"` queue is pre-created at `vxcore_context_create`, idle drain threads block on the queue's condvar (~0 CPU). There is no busy-spin and no need to guard against a missing queue.
128+
129+
**Degradation.** The initiating thread help-drains its own enqueued items, so a search stays correct even if this pool is absent or stalled. With no drain threads the search simply runs single-threaded; results, ordering, cancellation, and `max_results` are unaffected.

src/core/services/searchcoreservice.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ class SearchCoreService : public QObject, private Noncopyable {
7070
Error searchByTags(const QString &p_notebookId, const QString &p_queryJson,
7171
const QString &p_inputFilesJson, QJsonArray *p_results) const;
7272

73+
// Non-owning access to the vxcore context (owned in main(), outlives services).
74+
// Used by SearchService to drain the "vxcore.search" content-search work queue.
75+
VxCoreContextHandle context() const { return m_context; }
76+
7377
private:
7478
// Non-owning pointer to vxcore context.
7579
VxCoreContextHandle m_context = nullptr;

src/core/services/searchservice.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "searchservice.h"
22

3+
#include <algorithm>
4+
35
#include <QDebug>
46
#include <QJsonArray>
57
#include <QJsonObject>
@@ -186,13 +188,37 @@ SearchService::SearchService(SearchCoreService *p_coreService, QObject *p_parent
186188
});
187189

188190
m_thread->start();
191+
192+
unsigned n = std::thread::hardware_concurrency();
193+
if (n == 0) {
194+
n = 2;
195+
}
196+
n = std::min(n, 8u);
197+
198+
VxCoreContextHandle ctx = m_coreService->context();
199+
for (unsigned i = 0; i < n; ++i) {
200+
m_drainThreads.emplace_back([this, ctx]() {
201+
while (!m_stopDrain.load(std::memory_order_relaxed)) {
202+
if (vxcore_work_queue_process_next(ctx, "vxcore.search", 100) == 1) {
203+
m_drainItemsProcessed.fetch_add(1, std::memory_order_relaxed);
204+
}
205+
}
206+
});
207+
}
189208
}
190209

191210
SearchService::~SearchService() {
192211
cancel();
212+
m_stopDrain.store(true, std::memory_order_relaxed);
193213
m_thread->quit();
194214
m_thread->wait();
195215

216+
for (auto &t : m_drainThreads) {
217+
if (t.joinable()) {
218+
t.join();
219+
}
220+
}
221+
196222
delete m_mutex;
197223
m_mutex = nullptr;
198224
}

src/core/services/searchservice.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
#define SEARCHSERVICE_H
33

44
#include <atomic>
5+
#include <cstdint>
56
#include <memory>
7+
#include <thread>
8+
#include <vector>
69

710
#include <QMap>
811
#include <QObject>
@@ -12,6 +15,8 @@
1215
#include <core/error.h>
1316
#include <core/searchresulttypes.h>
1417

18+
#include <vxcore/vxcore.h>
19+
1520
class QMutex;
1621
class QThread;
1722

@@ -41,6 +46,11 @@ class SearchService : public QObject {
4146
bool isSearching() const;
4247
bool isSearching(int p_token) const;
4348

49+
size_t testDrainThreadCount() const { return m_drainThreads.size(); }
50+
uint64_t testDrainItemsProcessed() const {
51+
return m_drainItemsProcessed.load(std::memory_order_relaxed);
52+
}
53+
4454
signals:
4555
void searchFinished(int p_token, const SearchResult &p_result);
4656
void searchFailed(int p_token, const Error &p_error);
@@ -56,6 +66,10 @@ class SearchService : public QObject {
5666
QMap<int, std::shared_ptr<std::atomic<int>>> m_cancelFlags;
5767
QSet<int> m_activeTokens;
5868
int m_nextToken = 1;
69+
70+
std::vector<std::thread> m_drainThreads;
71+
std::atomic<bool> m_stopDrain{false};
72+
std::atomic<uint64_t> m_drainItemsProcessed{0};
5973
};
6074

6175
} // namespace vnotex

tests/core/CMakeLists.txt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,20 @@ add_qt_test(test_searchresulttypes
5252
GUILESS
5353
)
5454

55+
# T7 (search-drain-pool): SearchService owns a drain-thread pool that drains
56+
# vxcore's "vxcore.search" content-search work queue. Verifies (1) async
57+
# searchContent over a >50-file notebook matches the single-threaded baseline
58+
# result exactly, and (2) idle drain threads block on the condvar (0 items
59+
# processed across a 500ms idle window) with the pool sized min(hw,8) floor 2.
60+
add_qt_test(test_search_service_drain
61+
SOURCES
62+
test_search_service_drain.cpp
63+
LINKS
64+
core_services
65+
vxcore
66+
GUILESS
67+
)
68+
5569
add_qt_test(test_searchresultmodel
5670
SOURCES
5771
test_searchresultmodel.cpp

0 commit comments

Comments
 (0)