Skip to content

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

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
add-uos:fix-361631-add-cpu-cache-shared-cpu-list
May 20, 2026
Merged

fix(cpu): fix CPU cache size calculation with shared list#665
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
add-uos:fix-361631-add-cpu-cache-shared-cpu-list

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各级缓存总大小计算准确,系统监视器显示的缓存信息更准确。

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

lzwind
lzwind previously approved these changes May 20, 2026
@add-uos add-uos force-pushed the fix-361631-add-cpu-cache-shared-cpu-list branch from 818706e to 178ceeb Compare May 20, 2026 01:34
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各级缓存总大小计算准确,系统监视器显示的缓存信息更准确。
@add-uos add-uos force-pushed the fix-361631-add-cpu-cache-shared-cpu-list branch from 178ceeb to 752da2c Compare May 20, 2026 01:49
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已经仔细审查了你提供的Git Diff,该变更主要是为了读取并计算CPU缓存共享的CPU列表,从而更精确地计算总缓存大小。

总体来说,这个补丁的逻辑方向是正确的,特别是在计算L3 Cache总大小时,从原先的硬编码1改为根据shared_cpu_list动态计算,这是一个非常棒的改进!

不过,在代码质量、性能、健壮性和安全性方面,我发现了几个需要改进的地方。以下是我的详细审查意见:

1. 代码逻辑与健壮性

问题:parseSharedCpuCount 中单数字符串解析存在逻辑隐患

if (trimmed.toInt(&ok) || ok) {
    count += 1;
}
  • 解释toInt(&ok)在转换失败时返回0,且将ok置为false。如果输入是"0"toInt()返回0(在C++中相当于false),但oktrue。由于||短路特性,此时会依赖ok,结果正确。但如果输入是非法字符如"a"toInt()返回0okfalse0 || falsefalse,结果也正确。虽然当前逻辑碰巧能工作,但极其晦涩且依赖于Qt的隐式转换,阅读性很差。
  • 改进建议:直接判断ok即可。
bool ok = false;
trimmed.toInt(&ok);
if (ok) {
    count += 1;
}

问题:readCpuCacheIndex 中文件读取失败时依然赋值

QFile fileS(sharedPath);
if (fileS.open(QIODevice::ReadOnly)) {
    sharedCpuList = fileS.readAll();
}
fileS.close();
// ... 后续直接使用 sharedCpuList
  • 解释:如果文件不存在或无权限读取,sharedCpuList将为空字符串,后续代码会将空字符串设置到LogicalCpu中。虽然空字符串不会导致程序崩溃,但不够严谨。
  • 改进建议:建议在读取后对文件内容进行trimmed()处理,因为Linux sysfs文件通常带有末尾换行符。
if (fileS.open(QIODevice::ReadOnly)) {
    sharedCpuList = QString::fromUtf8(fileS.readAll()).trimmed();
}

2. 代码性能

问题:多余的 #include <QRegularExpression>

  • 解释:Diff显示在commonfunction.cpp中添加了#include <QRegularExpression>,但在代码中并没有使用正则表达式。引入不必要的头文件会增加编译时间。
  • 改进建议:删除该#include行。当前的基于splitcontains的字符串解析方式对于这个简单的格式已经足够且性能更好。

问题:readCpuCacheIndexreadAll() 后未去除换行符

  • 解释:与性能间接相关,如果不去除换行符,后续传给parseSharedCpuCount的字符串可能带有\n,虽然trimmed()能处理首尾,但在中间处理时可能产生空项。

3. 代码安全与异常处理

问题:除零风险

int groupCount = (sharedCount > 0 && logicalNum > 0) ? logicalNum / sharedCount : coreNum;
  • 解释:这里对logicalNum做了大于0的判断,防止了除零错误,做得很好!但是,对于L3 Cache的分支:
int groupCount = (sharedCount > 0 && logicalNum > 0) ? logicalNum / sharedCount : 1;
  • 改进建议:如果sharedCount为0(例如文件读取失败),回退值为1。这在逻辑上是合理的(L3通常全核共享),但建议增加一行日志输出,以便在出现意外情况时方便Debug。

问题:readCpuCacheIndex 中的文件句柄管理

  • 解释fileS.close()被放在了if块外面。如果fileS.open()失败,调用close()虽然不会报错(Qt内部会处理),但不符合RAII最佳实践。
  • 改进建议:利用Qt的QFile析构函数自动关闭文件的特性,或者将close()放入if块内。更推荐直接使用局部作用域:
QString sharedCpuList;
QFile fileS(sharedPath);
if (fileS.open(QIODevice::ReadOnly)) {
    sharedCpuList = QString::fromUtf8(fileS.readAll()).trimmed();
    fileS.close(); // 显式关闭,或者依赖析构
}

4. 代码风格与质量

问题:Getter方法缺少 const 修饰符

const QString &LogicalCpu::l1dSharedCpuList()  // 缺少 const
  • 解释:在C++中,获取成员变量且不修改对象状态的方法必须声明为const。这不仅符合语义,还能让该对象被const引用时也能调用该方法。原有的l4Cache()等方法似乎也缺少了const,但新加的方法应该修正这个问题。
  • 改进建议:头文件和源文件中的所有getter方法都应加上const
const QString &LogicalCpu::l1dSharedCpuList() const;

问题:版权时间修改的合理性

-// SPDX-FileCopyrightText: 2019 ~ 2023 UnionTech Software Technology Co., Ltd.
+// SPDX-FileCopyrightText: 2019 - 2026 UnionTech Software Technology Co., Ltd.
  • 解释:将结束年份直接改到未来的2026年。通常,版权声明的时间应该是代码实际创建/修改的年份,或者写成2019 - 至今。直接写未来的年份在法务上可能是不严谨的,建议与法务团队确认。

综合修改建议代码片段

针对 commonfunction.cpp 中的 parseSharedCpuCount,建议修改为:

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

    int count = 0;
    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); // 移除晦涩的 || ok 逻辑
            if (ok) {
                count += 1;
            }
        }
    }
    return count;
}

针对 logicalcpu.hlogicalcpu.cpp 中的 Getter:

// .h
const QString &l1dSharedCpuList() const;
const QString &l1iSharedCpuList() const;
const QString &l2SharedCpuList() const;
const QString &l3SharedCpuList() const;
const QString &l4SharedCpuList() const;

// .cpp
const QString &LogicalCpu::l1dSharedCpuList() const
{
    Q_D(const LogicalCpu); // 注意这里也要用 const
    return d->l1d_shared_cpu_list;
}
// ... 其他同理

针对 cpuinfo.cpp 中的文件读取:

// get shared_cpu_list
QString sharedCpuList;
QFile fileS(path + "/shared_cpu_list");
if (fileS.open(QIODevice::ReadOnly)) {
    sharedCpuList = QString::fromUtf8(fileS.readAll()).trimmed();
    fileS.close();
}

希望这些建议对你有所帮助!如果有任何疑问,欢迎继续讨论。

@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 8dec706 into linuxdeepin:master May 20, 2026
21 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