Repository navigation
perf(index): reduce resident anon RSS of incremental tasks - #291
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThe PR reduces idle resident memory by configuring glibc before worker threads start and by scheduling a debounced malloc_trim(0) after 30 seconds of task inactivity, while postponing reclamation during active work. Sequence diagram for debounced task idle heap trimmingsequenceDiagram
participant Task as IndexTask
participant Manager as TaskManager
participant Timer as m_idleTrimTimer
participant Allocator as glibc allocator
participant OS as Operating system
Task->>Manager: onTaskFinished()
Manager->>Manager: scheduleIdleTrim()
Manager->>Timer: start(30000)
opt another task finishes before timeout
Task->>Manager: onTaskPaused() or onTaskFinished()
Manager->>Manager: scheduleIdleTrim()
Manager->>Timer: start(30000)
end
Timer-->>Manager: timeout()
alt hasRunningTask()
Manager->>Timer: start(30000)
else no task running
Manager->>Allocator: malloc_trim(0)
Allocator->>OS: return free heap pages
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/index/main.cpp" line_range="93" />
<code_context>
+ // burst's peak. Fixed values (setting M_MMAP_THRESHOLD explicitly also
+ // disables the dynamic adjustment) keep big blocks on mmap where free
+ // returns pages to the OS immediately.
+ mallopt(M_ARENA_MAX, 2);
+ mallopt(M_MMAP_THRESHOLD, 2 * 1024 * 1024);
+ mallopt(M_TRIM_THRESHOLD, 256 * 1024);
</code_context>
<issue_to_address>
**Concurrent indexing becomes serialized**
When multiple profiles run allocation-heavy indexing tasks concurrently, `M_ARENA_MAX=2` caps the process to two glibc arenas, so allocation-heavy workers from concurrently active profiles must share arenas and contend on their allocator locks. Indexing throughput drops during concurrent profile workloads.
Benchmark the arena cap under concurrent profile workloads and set it high enough to avoid excessive allocator-lock contention while still meeting the RSS target.
</issue_to_address>
### Comment 2
<location path="src/index/task/taskmanager.h" line_range="154" />
<code_context>
QTimer *m_burstRecheckTimer { nullptr };
+
+ // 任务静默后的堆回收定时器(singleShot,见构造函数注释)
+ QTimer *m_idleTrimTimer { nullptr };
};
</code_context>
<issue_to_address>
**Private timer member misses required prefix**
When the header declares the new private timer member, `m_idleTrimTimer` uses the `m_` prefix instead of the required `_` prefix, and its mixed-case name violates the snake_case naming rule.
Rename the member and its references to `_idle_trim_timer`.
</issue_to_address>
### Comment 3
<location path="src/index/task/taskmanager.cpp" line_range="49" />
<code_context>
+// 任务静默期:任务结束后延迟多久把 glibc 堆里残留的空闲页还给 OS。
+// 取值需覆盖"事件批连发"的典型间隔(collector 窗口 1~3s),避免与
+// 下一批任务的分配持续互相抵消;正在跑任务时定时器会自行顺延。
+constexpr int kIdleTrimDelayMs = 30000;
+
TaskQueueItem createCompensationTaskItem(const QStringList &paths)
</code_context>
<issue_to_address>
**Allocator constant breaks naming rule**
`kIdleTrimDelayMs` uses mixed case instead of the required UPPER_CASE, so the new constant breaks the repository naming rule.
Rename the constant to UPPER_CASE.
</issue_to_address>| // burst's peak. Fixed values (setting M_MMAP_THRESHOLD explicitly also | ||
| // disables the dynamic adjustment) keep big blocks on mmap where free | ||
| // returns pages to the OS immediately. | ||
| mallopt(M_ARENA_MAX, 2); |
There was a problem hiding this comment.
🟡 Medium · Concurrent indexing becomes serialized
When multiple profiles run allocation-heavy indexing tasks concurrently, M_ARENA_MAX=2 caps the process to two glibc arenas, so allocation-heavy workers from concurrently active profiles must share arenas and contend on their allocator locks. Indexing throughput drops during concurrent profile workloads.
Benchmark the arena cap under concurrent profile workloads and set it high enough to avoid excessive allocator-lock contention while still meeting the RSS target.
Prompt for AI agents
In `src/index/main.cpp` at line 93:
**Concurrent indexing becomes serialized**
When multiple profiles run allocation-heavy indexing tasks concurrently, `M_ARENA_MAX=2` caps the process to two glibc arenas, so allocation-heavy workers from concurrently active profiles must share arenas and contend on their allocator locks. Indexing throughput drops during concurrent profile workloads.
Benchmark the arena cap under concurrent profile workloads and set it high enough to avoid excessive allocator-lock contention while still meeting the RSS target.| // 任务静默期:任务结束后延迟多久把 glibc 堆里残留的空闲页还给 OS。 | ||
| // 取值需覆盖"事件批连发"的典型间隔(collector 窗口 1~3s),避免与 | ||
| // 下一批任务的分配持续互相抵消;正在跑任务时定时器会自行顺延。 | ||
| constexpr int kIdleTrimDelayMs = 30000; |
There was a problem hiding this comment.
⚪ Low · Allocator constant breaks naming rule
kIdleTrimDelayMs uses mixed case instead of the required UPPER_CASE, so the new constant breaks the repository naming rule.
Rename the constant to UPPER_CASE.
Prompt for AI agents
In `src/index/task/taskmanager.cpp` at line 49:
**Allocator constant breaks naming rule**
`kIdleTrimDelayMs` uses mixed case instead of the required UPPER_CASE, so the new constant breaks the repository naming rule.
Rename the constant to UPPER_CASE.From AGENTS.md L60:
- Constants: `UPPER_CASE` (e.g., `MAX_INPUT_MINOR`, `INFO_LEVELS`)…undary trim Incremental index tasks allocate and free large transients in bursts (whole-file wchar copies, Lucene analyzer/posting buffers, per-batch reader/writer). Under glibc defaults those freed pages stay resident, so idle RSS sits at the last burst's high-water (~200-360MB observed, dropping only under a manual malloc_trim). Two measures: - pin allocator params at startup: M_ARENA_MAX=2, fixed M_MMAP_THRESHOLD=2MB (an explicit value also disables glibc's dynamic threshold raise), M_TRIM_THRESHOLD=256KB - return the arena heap at task boundaries: when no task is running and mallinfo2().fordblks >= 32MB, call malloc_trim(0), at most once per 10s. An idle-window debounce was tried first and never fires on hosts with steady incremental churn (events arrive every few seconds, and long commit/merge phases keep the task marker set), so the trigger is based on reclaimable heap size instead. Log the per-task heap state (free/in-use/RSS) and the trim result at debug level, so RSS attribution stays verifiable in the field. Verified on a 5.3GB fulltext index: a 30-file batch leaves 85MB free / 28MB in use at 148MB RSS and trim brings RSS to 67MB; repeated batches 128->70MB and 100->71MB. Steady state 214MB -> 71MB.
a8a3030 to
5186a07
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 语法正确,逻辑清晰 ✅ 通过 潜在问题: 建议: ['无需修改,逻辑清晰正确。'] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: ['将 maybeTrimHeap() 声明从 Q_SLOTS 区域移至 private: 区域,保持声明语义与实际用途一致。'] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: ['无需修改,性能设计合理。'] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: ['无需修改,安全合规。'] 💡 改进建议代码示例// taskmanager.h 修复:将 maybeTrimHeap() 移至 private 区域
private:
qint64 queuedIncrementalCount() const;
void updateBacklogState();
void recordIngestedFiles(qint64 count);
// 任务边界处的条件式堆回收(见实现处注释)
void maybeTrimHeap();本报告由 AI 代码审查工具自动生成 |
|
/forcemerge |
Incremental index tasks allocate and free large transients in bursts
(whole-file wchar copies, Lucene analyzer/posting buffers). Under glibc
defaults those freed pages stay resident: the dynamic mmap threshold
rises after the first big munmap, per-task worker QThreads each hold
their own arena at high-water, so idle RSS lingers near the last
burst's peak (~200MB observed, reclaimable via malloc_trim).
Two measures:
M_MMAP_THRESHOLD=2MB (disables dynamic raise), M_TRIM_THRESHOLD=256KB
deferred while another task is running
Summary by Sourcery
Reduce idle RSS from incremental indexing by tuning glibc allocation and conditionally trimming the heap at task boundaries.
Bug Fixes:
Enhancements: