Skip to content

perf: cache resolved indexed directories to avoid repeated resolution#294

Merged
Johnson-zs merged 1 commit into
linuxdeepin:masterfrom
liyigang1:master
May 21, 2026
Merged

perf: cache resolved indexed directories to avoid repeated resolution#294
Johnson-zs merged 1 commit into
linuxdeepin:masterfrom
liyigang1:master

Conversation

@liyigang1
Copy link
Copy Markdown
Contributor

Made the dirs variable in defaultIndexedDirectory() static const so that getResolvedIndexedDirectories() is only called once. This avoids redundant directory resolution on every invocation of the default indexed directory lookup.

defaultIndexedDirectory()中的dirs变量声明为static const, 使getResolvedIndexedDirectories()仅被调用一次,避免每次查找默认
索引目录时重复进行目录解析。

Log: 缓存已解析的索引目录避免重复解析
PMS: https://pms.uniontech.com/bug-view-361809.html
Influence: 减少重复调用时的冗余文件系统操作,提升频繁查询默认索引目录的搜索场景性能。需确认应用重启后仍能正确加载索引目录配置变更,以及静态初始化不存在线程安全问题。

Johnson-zs
Johnson-zs previously approved these changes May 21, 2026
Made the `dirs` variable in `defaultIndexedDirectory()` static const so
that `getResolvedIndexedDirectories()` is only called once. This avoids
redundant directory resolution on every invocation of the default indexed
directory lookup.

将`defaultIndexedDirectory()`中的`dirs`变量声明为static const,
使`getResolvedIndexedDirectories()`仅被调用一次,避免每次查找默认
索引目录时重复进行目录解析。

Log: 缓存已解析的索引目录避免重复解析
PMS: https://pms.uniontech.com/bug-view-361809.html
Influence: 减少重复调用时的冗余文件系统操作,提升频繁查询默认索引目录的搜索场景性能。需确认应用重启后仍能正确加载索引目录配置变更,以及静态初始化不存在线程安全问题。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改的核心是将多处 static const QStringList &(静态常量引用)改为了 static const QStringList(静态常量局部变量),以及一处将普通引用改为了静态局部变量。

整体而言,这次修改非常合理且必要,主要修复了严重的悬垂引用逻辑缺陷问题。以下是详细的审查意见:

1. 语法与逻辑审查 (⭐⭐⭐⭐⭐ 优秀)

1.1 修复悬垂引用 - 第615行 和 第673行

  • 修改前static const QStringList &kDirs = DFMSEARCH::Global::defaultIndexedDirectory();
  • 分析defaultIndexedDirectory() 函数返回的是 QStringList(右值)。将一个右值绑定到 const QStringList & 在C++语法上是合法的(常量引用可以延长临时对象的生命周期),其生命周期仅维持到当前完整表达式结束。然而,加上了 static 关键字后,情况就变了。static 引用只在首次执行时初始化,它试图让引用的生命周期延长至整个程序。但被绑定的临时对象(defaultIndexedDirectory() 返回的匿名对象)在表达式结束后就会被销毁。这就导致 kDirs 成了一个悬垂引用,后续对该变量的访问将触发未定义行为,极大概率导致程序崩溃或数据错乱。
  • 修改后static const QStringList kDirs = ...
  • 结论:去掉引用符号 & 后,kDirs 成为一个真正的静态局部对象,它会将返回的临时对象拷贝/移动构造到 kDirs 中,从而安全地存在于整个程序生命周期。这是一个关键的Bug修复。

1.2 修复逻辑与生命周期问题 - 第514行

  • 修改前const QStringList &dirs = getResolvedIndexedDirectories();
  • 分析:这里没有 static,但同样存在引用绑定临时对象的问题。虽然C++规定常量引用可以延长临时对象的生命周期至引用本身的生命周期,但这是C++中极其容易出错的语法特性。如果 getResolvedIndexedDirectories() 未来被修改为返回一个非 const 的引用,或者代码重构时不小心破坏了生命周期保证,极易引发悬垂引用。
  • 修改后static const QStringList dirs = getResolvedIndexedDirectories();
  • 结论:加上 static 并去掉 &,不仅彻底消除了临时对象生命周期隐患,还利用了 static 的惰性初始化特性,使得该函数在多次调用时只需计算一次,提升了性能。

1.3 修复黑名单逻辑 - 第714行

  • 修改前static const QStringList &kBlackPaths = defaultBlacklistPaths();
  • 分析:同1.1,存在悬垂引用问题。此外,作为黑名单,如果使用 static,意味着程序运行期间黑名单是固定的。如果黑名单需要动态更新,这里的 static 会导致更新失效。
  • 修改后static const QStringList kBlackPaths = defaultBlacklistPaths();
  • 结论:修复了悬垂引用崩溃问题。如果黑名单确实不需要动态更新,则逻辑正确。

2. 代码质量审查 (⭐⭐⭐⭐ 良好)

  • 避免返回类型的过度绑定:在C++中,函数返回值(尤其是像 QStringList 这样的值类型)应该直接用对象去接收,而不是用引用去接收。使用 const Type & 接收返回值是过度优化,且极易引发生命周期Bug。本次修改统一去掉了接收返回值时的 &,符合现代C++的最佳实践(RVO/NRVO和移动语义已经让值返回非常高效)。
  • 命名规范kDirskBlackPaths 使用了 k 前缀表示常量,符合常见的C++命名规范,代码可读性良好。

3. 代码性能审查 (⭐⭐⭐⭐ 良好)

  • 静态初始化的收益:将 dirs 改为 static,避免了每次调用 defaultIndexedDirectory() 时重复调用 getResolvedIndexedDirectories(),这是一个明显的性能提升。
  • 拷贝与移动开销:去掉 & 意味着会触发一次拷贝或移动构造。但由于 defaultIndexedDirectory() 返回的是右值,现代C++编译器会优先触发移动语义QStringList 的移动操作是 O(1) 的指针交换),开销极小。相比于每次都重新计算目录列表,这点移动开销微不足道。

4. 代码安全审查 (⭐⭐⭐⭐⭐ 优秀)

  • 内存安全:本次修改最核心的贡献是消除了多处悬垂引用导致的内存安全问题。在并发或复杂调用链中,悬垂引用引发的崩溃往往难以复现和定位,修改后代码的内存安全性得到了根本性保障。

💡 综合改进建议

虽然当前修改已经非常出色,但仍有微调的空间,以应对未来可能的需求变更:

  1. 关于 static 的动态更新问题
    使用 static 局部变量意味着这些索引目录和黑名单在程序启动后首次调用时被缓存,之后永不更新。如果这些配置项在程序运行期间需要支持热更新(例如用户在设置中修改了索引目录),当前的 static 会导致更新无法生效。

    • 建议:如果这些配置确实是只读的、全局不变的,当前代码完美。如果未来需要支持动态更新,建议将这些 static 局部变量提取为类的成员变量,或者使用 std::shared_ptr 配合原子操作/读写锁来实现可刷新的配置缓存。
  2. 关于 defaultIndexedDirectory() 函数本身的设计
    当前 defaultIndexedDirectory() 内部使用了 static 缓存,外部调用时又将其结果赋值给另一个 static 变量(如 isPathInContentIndexDirectory 中的 kDirs)。这实际上造成了双重缓存,略显冗余。

    • 建议:既然 defaultIndexedDirectory() 内部已经保证了只计算一次,外部调用时完全可以直接使用普通的局部变量,无需再加 static
      const QStringList kDirs = DFMSEARCH::Global::defaultIndexedDirectory();
      这样既保证了性能(因为 defaultIndexedDirectory() 内部是静态的),又减少了外部逻辑的复杂度,还为未来可能的动态刷新留出了余地(只需刷新 defaultIndexedDirectory() 内部的静态变量即可)。

总结:本次Diff是一次高质量的修复,成功消除了由 static const T & 绑定右值带来的严重悬垂引用Bug。代码在安全性、性能和逻辑正确性上均有显著提升。可以直接合入。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs, liyigang1

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Johnson-zs Johnson-zs merged commit 4226c18 into linuxdeepin:master May 21, 2026
21 checks passed
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.

3 participants