Skip to content

fix(cpu): fix CPU cache size calculation with shared list#666

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/eaglefrom
add-uos:develop/eagle
May 20, 2026
Merged

fix(cpu): fix CPU cache size calculation with shared list#666
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/eaglefrom
add-uos:develop/eagle

Conversation

@add-uos
Copy link
Copy Markdown
Contributor

@add-uos add-uos commented May 20, 2026

Add support for reading CPU cache shared_cpu_list from sysfs and use it to calculate total cache size accurately for each cache level.

添加对CPU缓存shared_cpu_list的支持,从sysfs读取并用于准确
计算各级缓存总大小。

Log: 修复CPU缓存大小计算错误
PMS: BUG-361631
Influence: 修复后CPU各级缓存总大小计算准确,系统监视器显示的缓存信息更准确。
Change-Id: Id22007c9becf0e1e06f3203386410669c94c4d6f

Add support for reading CPU cache shared_cpu_list from sysfs and use
it to calculate total cache size accurately for each cache level.

添加对CPU缓存shared_cpu_list的支持,从sysfs读取并用于准确
计算各级缓存总大小。

Log: 修复CPU缓存大小计算错误
PMS: BUG-361631
Influence: 修复后CPU各级缓存总大小计算准确,系统监视器显示的缓存信息更准确。
Change-Id: Id22007c9becf0e1e06f3203386410669c94c4d6f
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @add-uos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已经仔细审查了你提供的Git Diff代码。本次代码变更的主要目的是读取并计算CPU缓存共享的CPU列表,以此来更精确地计算总缓存大小,而不是像以前那样简单粗暴地用核心数或固定值1去乘。

整体思路非常正确,但在代码安全、逻辑严谨性、代码性能和规范方面,我发现了一些需要改进的地方。以下是详细的审查意见:

1. 代码安全

问题:文件读取未做错误状态检查,且存在资源泄漏风险

  • 位置cpuinfo.cpp 第377-382行
    QFile fileS(sharedPath);
    if (fileS.open(QIODevice::ReadOnly)) {
        sharedCpuList = fileS.readAll();
    }
    fileS.close();
  • 分析
    1. fileS.readAll() 读取后没有对读取的内容进行校验,如果文件内容巨大或读取中断,可能会引入异常数据。
    2. 虽然使用了 open() 并在后面调用了 close(),但如果在 open()close() 之间抛出异常,文件描述符将泄漏。
  • 改进建议:使用 Qt 的 RAII 模式(栈上对象自动销毁),或至少确保异常安全。另外,建议读取后对字符串进行清理(去除末尾换行符等)。
    QFile fileS(sharedPath);
    if (fileS.open(QIODevice::ReadOnly | QIODevice::Text)) {
        sharedCpuList = fileS.readAll().trimmed(); // 去除可能的换行符和空格
        // fileS.close(); // Qt的QFile析构时会自动关闭,无需显式调用
    }

2. 语法逻辑

问题1:整数解析逻辑存在严重Bug

  • 位置commonfunction.cpp 第347-349行
    bool ok = false;
    if (trimmed.toInt(&ok) || ok) {
        count += 1;
    }
  • 分析:这个逻辑是错误的。QString::toInt(&ok) 如果字符串不是数字,会返回 0,并且将 ok 置为 false
    • 假设 trimmed"abc"toInt(&ok) 返回 0okfalse。条件 0 || falsefalse,正确跳过。
    • 假设 trimmed"0"toInt(&ok) 返回 0oktrue。条件 0 || truetrue,正确计数。
    • 但是,这种写法极其反直觉且依赖于 toInt 返回 0 的特性,如果以后有人修改了默认返回值,这里就会崩溃。正确的逻辑应该只检查 ok
  • 改进建议
    bool ok = false;
    int val = trimmed.toInt(&ok);
    if (ok) {
        count += 1;
    }

问题2:QStringList::SkipEmptyParts 在 Qt 5.14+ 中已被弃用

  • 位置commonfunction.cpp 第329行
    QStringList parts = s.split(',', QString::SkipEmptyParts);
  • 分析QString::SplitBehavior 在 Qt 5.14 中被标记为弃用,在 Qt 6 中已被移除。为了代码的向前兼容性,应使用新的枚举 Qt::SkipEmptyParts
  • 改进建议
    QStringList parts = s.split(',', Qt::SkipEmptyParts);

问题3:除法运算潜在的整数除零风险

  • 位置DeviceGenerator.cpp 第1288、1296、1304、1312行
    int groupCount = (sharedCount > 0 && logicalNum > 0) ? logicalNum / sharedCount : coreNum;
  • 分析:虽然你已经通过 sharedCount > 0 防止了除以0,但 logicalNum / sharedCount 是整数除法。如果 logicalNum 不能被 sharedCount 整除(例如大核小核架构,或者系统获取的逻辑CPU数本身就不准),groupCount 会被截断,导致计算出的总缓存偏小。
  • 改进建议:建议使用四舍五入或向上取整,确保缓存计算不丢失。同时,对于 L3 缓存,原本代码强制写死为 1,现在改为动态计算是好的,但 L3 通常确实是所有核心共享的,如果 sharedCount == logicalNumgroupCount 就是1,逻辑自洽;但建议加个注释说明为何 L3 的 fallback 值是 1。

3. 代码性能

问题:引入了不必要的重量级正则表达式头文件

  • 位置commonfunction.cpp 第11行
    #include <QRegularExpression>
  • 分析:在新增的 parseSharedCpuCount 函数中,你只使用了 QString::split()QString::contains()QString::toInt()完全没有使用正则表达式QRegularExpression 的包含会增加编译时间,且没有任何作用。
  • 改进建议:删除 #include <QRegularExpression>

4. 代码质量与规范

问题1:版权声明格式不统一

  • 位置commonfunction.cppcommonfunction.h 的头部
    -// SPDX-FileCopyrightText: 2022 - 2026 UnionTech Software Technology Co., Ltd.
    +// Copyright (C) 2019 ~ 2026 Uniontech Software Technology Co.,Ltd.
    +// SPDX-FileCopyrightText: 2019 - 2026 UnionTech Software Technology Co., Ltd.
  • 分析:你同时添加了传统版权声明 Copyright (C) 和标准化版权声明 SPDX-FileCopyrightText,这造成了冗余。而且其他文件(如 corecpu.cpp)只修改了 SPDX-FileCopyrightText。应保持整个项目版权声明风格的一致性。
  • 改进建议:只保留 SPDX-FileCopyrightText 即可,符合开源规范。

问题2:Getter 函数缺少 const 修饰符

  • 位置logicalcpu.h 第189-193行
    const QString &l1dSharedCpuList();
    const QString &l1iSharedCpuList();
    // ...
  • 分析:在 C++ 中,不修改对象状态的访问器函数必须声明为 const 成员函数。否则,你将无法在一个 const LogicalCpu& 对象上调用这些方法。原有的 l4Cache() 等也缺少 const,但这不应该成为新代码继续犯错的理由。
  • 改进建议
    const QString &l1dSharedCpuList() const;
    const QString &l1iSharedCpuList() const;
    const QString &l2SharedCpuList() const;
    const QString &l3SharedCpuList() const;
    const QString &l4SharedCpuList() const;

问题3:L4 shared cpu list 等输出键名风格不一致

  • 位置corecpu.cpp 第66-70行
    appendKeyValue(info, "L1d shared cpu list", logical.l1dSharedCpuList());
  • 分析:原有的键名如 "L2 cache", "CPU MHz" 首字母大写且无多余空格,新增的 "L1d shared cpu list" 全小写且较长,在 UI 显示上可能不够美观和统一。
  • 改进建议:建议与项目产品或UI设计确认,是否改为 "L1d Shared CPU List""L1d Shared Cpus" 等更规范的格式。

综合修改建议代码片段

针对 commonfunction.cpp 中的 parseSharedCpuCount,建议修改为如下安全、规范的实现:

int Common::parseSharedCpuCount(const QString &sharedCpuList)
{
    QString s = sharedCpuList.trimmed();
    if (s.isEmpty())
        return 0;

    int count = 0;
    // 使用 Qt5.14+ 的新枚举,兼容 Qt6
    QStringList parts = s.split(',', Qt::SkipEmptyParts);
    for (const QString &part : parts) {
        QString trimmed = part.trimmed();
        if (trimmed.contains('-')) {
            QStringList range = trimmed.split('-');
            if (range.size() == 2) {
                bool ok1 = false, ok2 = false;
                int start = range[0].trimmed().toInt(&ok1);
                int end = range[1].trimmed().toInt(&ok2);
                if (ok1 && ok2 && end >= start) {
                    count += end - start + 1;
                }
            }
        } else {
            bool ok = false;
            trimmed.toInt(&ok); // 只做转换测试
            if (ok) {           // 只检查 ok 状态
                count += 1;
            }
        }
    }
    return count;
}

希望这些审查意见对你有帮助!如果有任何疑问,欢迎随时提问。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, lzwind

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

@add-uos
Copy link
Copy Markdown
Contributor Author

add-uos commented May 20, 2026

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 20, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit a0791da into linuxdeepin:develop/eagle May 20, 2026
20 of 22 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