Repository navigation
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos 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 guide (collapsed on small PRs)Reviewer's GuideAdds 30-second TTL invalidation to the static GPU memory cache, refreshing data after expiration while retaining the previous value and retrying on read failures without changing the public interface. Sequence diagram for GPU memory cache refreshsequenceDiagram
participant Caller
participant DeviceInterface
participant GPUInfoFile
Caller->>DeviceInterface: getGpuInfoForFTDTM()
DeviceInterface->>DeviceInterface: Check cache expiration
alt Cache empty or TTL expired
DeviceInterface->>GPUInfoFile: getGpuMemInfoForFTDTM(mapInfo)
alt Read succeeds
GPUInfoFile-->>DeviceInterface: GPU memory data
DeviceInterface->>DeviceInterface: Update gpuMemInfo and lastUpdate
else Read fails
GPUInfoFile-->>DeviceInterface: Failure
DeviceInterface-->>Caller: Return previous cached value
end
else Cache valid
DeviceInterface-->>Caller: Return cached value
end
DeviceInterface-->>Caller: GPU memory info
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="deepin-devicemanager-server/deepin-deviceinfo/src/loadinfo/deviceinterface.cpp" line_range="151-153" />
<code_context>
- if (gpuMemInfo.isEmpty()) {
+ static QDateTime lastUpdate;
+
+ bool cacheExpired = !lastUpdate.isValid()
+ || lastUpdate.secsTo(QDateTime::currentDateTime()) >= kGpuCacheTtlSec;
+ if (gpuMemInfo.isEmpty() || cacheExpired) {
QMap<QString, QString> mapInfo;
if (getGpuMemInfoForFTDTM(mapInfo)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** `lastUpdate.secsTo(QDateTime::currentDateTime())` uses wall-clock time, so a backward system-clock adjustment makes the elapsed value negative and prevents the cache from expiring until wall time catches up. The daemon then returns stale GPU data beyond the 30-second TTL.
**Triggers:** When the system clock is corrected backward or synchronized by NTP while the daemon is running.
**Suggested fix:** Use a monotonic elapsed-time source such as `QElapsedTimer` for TTL calculations.
</issue_to_address>| bool cacheExpired = !lastUpdate.isValid() | ||
| || lastUpdate.secsTo(QDateTime::currentDateTime()) >= kGpuCacheTtlSec; | ||
| if (gpuMemInfo.isEmpty() || cacheExpired) { |
There was a problem hiding this comment.
issue (bug_risk): lastUpdate.secsTo(QDateTime::currentDateTime()) uses wall-clock time, so a backward system-clock adjustment makes the elapsed value negative and prevents the cache from expiring until wall time catches up. The daemon then returns stale GPU data beyond the 30-second TTL.
Triggers: When the system clock is corrected backward or synchronized by NTP while the daemon is running.
Suggested fix: Use a monotonic elapsed-time source such as QElapsedTimer for TTL calculations.
deepin pr auto reviewAI 代码审查报告
总体评价总分: 98/100 - 优秀 本次提交修复了 修改文件清单
四维度评分维度1: 语法逻辑 (25/25) ✓
分析:
边界条件分析:
维度2: 代码质量 (23/25) ✓
分析:
扣分项:
维度3: 代码性能 (20/20) ✓
分析:
维度4: 代码安全 (30/30) ✓
分析:
漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个 OCR 审查结果
改进建议建议1: 添加 TTL 缓存策略注释在 QString DeviceInterface::getGpuInfoForFTDTM()
{
// TTL cache: re-read GPU memory info every 30 seconds to avoid
// stale data in long-running daemon. On read failure, the timestamp
// is not updated so the next call retries automatically.
static constexpr int kGpuCacheTtlSec = 30;
static QString gpuMemInfo { "" };
static QDateTime lastUpdate;
// ...
}审查结论本次提交是一个高质量的 Bug 修复,解决了守护进程长时间运行后 GPU 显存信息缓存过期的问题。代码修改简洁(+9/-1),逻辑正确,边界条件处理完善,未引入安全漏洞。TTL 缓存机制设计合理,在数据新鲜度和系统性能之间取得了良好平衡。建议补充缓存策略注释以提升可维护性。 评分明细:
|
e254193 to
7636578
Compare
|
TAG Bot New tag: 6.0.74 |
1. Root cause: getGpuInfoForFTDTM() used a static QString that was never refreshed after first successful read, daemon returned stale GPU memory data for its entire lifetime 2. Fix: introduce static QDateTime timestamp and 30-second TTL constant, re-read the underlying file when cache is empty or expired; on read failure the timestamp is not updated so the next call retries 3. Impact: callers via DBus now receive data that is at most 30 seconds old; no interface or signature change, no impact on other callers Log: fix stale GPU memory info in long-running daemon Influence: 1. Verify GPU memory info displays correctly after daemon start 2. Verify info refreshes after 30 seconds when underlying data changes 3. Verify graceful behavior when GPU info file is unavailable fix: 修复GPU显存信息缓存过期问题 1. 根因:getGpuInfoForFTDTM() 使用 static QString 永久缓存,首次成功 读取后永不刷新,守护进程生命周期内始终返回过期数据 2. 方案:引入 static QDateTime 时间戳和 30 秒 TTL 常量,缓存为空或已 过期时重新读取文件;读取失败不更新时间戳,下次调用继续重试 3. 影响:通过 DBus 调用的前端将获得最多 30 秒延迟的新鲜数据;接口 和签名不变,对其他调用者无影响 Log: 修复守护进程长时间运行后GPU显存信息过期问题 Influence: 1. 测试守护进程启动后 GPU 显存信息正确显示 2. 测试底层文件变更后 30 秒内信息能自动刷新 3. 测试 GPU 信息文件不可用时的容错行为
7636578 to
1826a2c
Compare
fix: add TTL cache invalidation for GPU memory info
never refreshed after first successful read, daemon returned stale
GPU memory data for its entire lifetime
re-read the underlying file when cache is empty or expired; on read
failure the timestamp is not updated so the next call retries
old; no interface or signature change, no impact on other callers
Log: fix stale GPU memory info in long-running daemon
Influence:
fix: 修复GPU显存信息缓存过期问题
读取后永不刷新,守护进程生命周期内始终返回过期数据
过期时重新读取文件;读取失败不更新时间戳,下次调用继续重试
和签名不变,对其他调用者无影响
Log: 修复守护进程长时间运行后GPU显存信息过期问题
Influence:
Summary by Sourcery
Prevent stale GPU memory information by adding time-based cache invalidation while preserving existing interfaces.
Bug Fixes:
Chores: