Fix: [gpu-info] The gpu mem not show. #543
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
-- Adjust code logic to show gpu mem. Log: fix issue Bug: https://pms.uniontech.com/bug-view-336355.html
Reviewer's GuideThis PR replaces the legacy external GPU info commands with a direct sysfs reader for GPU memory, exposes it via a new D-Bus method, and integrates it into the existing GPU info flow while removing obsolete code. Sequence diagram for GPU memory info retrieval via D-BussequenceDiagram
participant App
participant CommonTools
participant QDBusInterface
participant DeviceInfoServer
participant DeviceInterface
App->>CommonTools: preGenerateGpuInfo()
CommonTools->>QDBusInterface: call("getGpuInfoForFTDTM")
QDBusInterface->>DeviceInfoServer: D-Bus call getGpuInfoForFTDTM
DeviceInfoServer->>DeviceInterface: getGpuMemInfoForFTDTM(mapInfo)
DeviceInterface->>DeviceInterface: Read /sys/kernel/debug/gc/total_mem
DeviceInterface->>DeviceInterface: Parse and format memory info
DeviceInterface-->>DeviceInfoServer: Return GPU memory info
DeviceInfoServer-->>QDBusInterface: Return GPU memory info
QDBusInterface-->>CommonTools: Return GPU memory info
CommonTools-->>App: Return combined GPU info
Class diagram for updated DeviceInterface and related GPU info retrievalclassDiagram
class DeviceInterface {
+setMonitorDeviceFlag(bool flag)
+getGpuInfoForFTDTM()
-getUserAuthorPasswd()
-getGpuMemInfoForFTDTM(QMap<QString, QString> &mapInfo)
}
class CommonTools {
+preGenerateGpuInfo()
-getGpuBaseInfo(QMap<QString, QString> &mapInfo)
}
class QDBusInterface {
+call(method)
}
DeviceInterface <|-- DeviceInfoServer
CommonTools --> QDBusInterface : uses
QDBusInterface --> DeviceInfoServer : D-Bus call
DeviceInfoServer --> DeviceInterface : calls
CommonTools --> DeviceInterface : via D-Bus
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review根据提供的代码差异，我来对代码进行审查，并提出改进意见：
具体改进建议：
bool DeviceInterface::getGpuMemInfoForFTDTM(QMap<QString, QString> &mapInfo)
{
const QString filePath = "/sys/kernel/debug/gc/total_mem";
QFile file(filePath);
if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
qCritical() << "Error opening file:" << file.errorString();
return false;
}
QTextStream in(&file);
QString content = in.readAll();
file.close();
if (content.isEmpty()) {
qCritical() << "Error: File is empty!";
return false;
}
// 其余代码...
}
static const QRegularExpression kMemInfoRegex(R"((\d+(?:\.\d+)?)\s*\(?(MB|GB|KB|B)\)?)",
QRegularExpression::CaseInsensitiveOption);
bool DeviceInterface::getGpuMemInfoForFTDTM(QMap<QString, QString> &mapInfo)
{
// ... 前面的文件操作代码 ...
QRegularExpressionMatch match = kMemInfoRegex.match(content.trimmed());
if (!match.hasMatch()) {
qCritical() << "Error: Failed to parse memory info";
return false;
}
// ... 其余代码 ...
}
QString DeviceInterface::convertToGB(double value, const QString &unit)
{
static const QMap<QString, double> unitFactors = {
{"B", 1.0 / (1024.0 * 1024.0 * 1024.0)},
{"KB", 1.0 / (1024.0 * 1024.0)},
{"MB", 1.0 / 1024.0},
{"GB", 1.0}
};
auto it = unitFactors.find(unit.toUpper());
if (it == unitFactors.end()) {
qCritical() << "Error: Unsupported unit:" << unit;
return QString();
}
return QString("%1GB").arg(value * it.value(), 0, 'f', 2);
}
这些改进将提高代码的健壮性、可维护性和性能。同时，良好的错误处理和日志记录将有助于问题的快速定位和解决。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Add an explicit fallback or error branch in getGpuMemInfoForFTDTM for unrecognized units so totalValue doesn’t remain empty if the regex captures an unexpected unit.
- Static caching of GPU info means it will never refresh at runtime; consider adding a cache invalidation or refresh trigger to pick up memory updates after the first call.
- Both DeviceInterface and CommonTools contain similar loops to build the key:value strings from a QMap—extract that into a shared helper function to reduce code duplication.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add an explicit fallback or error branch in getGpuMemInfoForFTDTM for unrecognized units so totalValue doesn’t remain empty if the regex captures an unexpected unit.
- Static caching of GPU info means it will never refresh at runtime; consider adding a cache invalidation or refresh trigger to pick up memory updates after the first call.
- Both DeviceInterface and CommonTools contain similar loops to build the key:value strings from a QMap—extract that into a shared helper function to reduce code duplication.
## Individual Comments
### Comment 1
<location> `deepin-devicemanager-server/deepin-deviceinfo/src/loadinfo/deviceinterface.cpp:32` </location>
<code_context>
return result == Authority::Yes;
}
+bool DeviceInterface::getGpuMemInfoForFTDTM(QMap<QString, QString> &mapInfo)
+{
+ const QString filePath = "/sys/kernel/debug/gc/total_mem";
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the method by extracting helpers, using a unit conversion map, and simplifying formatting with join().
Here are a few low-risk refactorings you can apply to keep the exact same behaviour but break that monster method into bite-sized helpers, remove the deep if/else, and collapse your formatting loop into a single `join()`.
1) pull the file-read and empty-check out into its own helper:
```cpp
static QString readFileContent(const QString& path) {
QFile f(path);
if (!f.open(QIODevice::ReadOnly|QIODevice::Text)) {
qCritical() << "Error opening" << path << ":" << f.errorString();
return {};
}
return QString::fromUtf8(f.readAll()).trimmed();
}
```
2) pull the regex match into a helper + make the regex `static const` so it isn’t reallocated every call:
```cpp
namespace {
static const QRegularExpression kMemRx{
R"((\d+(?:\.\d+)?)\s*\(?(MB|GB|KB|B)\)?)",
QRegularExpression::CaseInsensitiveOption
};
}
bool parseMemInfo(const QString& content, double& outValue, QString& outUnit) {
auto m = kMemRx.match(content);
if (!m.hasMatch()) {
qCritical() << "Error: no memory info in:" << content;
return false;
}
outValue = m.captured(1).toDouble();
outUnit = m.captured(2).toUpper();
return true;
}
```
3) replace the big `if/else` chain with a simple unit→factor map:
```cpp
static const QHash<QString,double> kUnitToGb {
{ "B", 1.0/(1024*1024*1024) },
{ "KB", 1.0/(1024*1024) },
{ "MB", 1.0/1024.0 },
{ "GB", 1.0 }
};
QString convertToGbString(double value, const QString& unit) {
double factor = kUnitToGb.value(unit, 0);
if (factor == 0) {
qCritical() << "Unknown unit:" << unit;
return {};
}
return QString::number(value * factor, 'f', 2) + "GB";
}
```
4) re-write `getGpuMemInfoForFTDTM()` as a simple orchestrator, and replace your manual loop in `getGpuInfoForFTDTM()` with a `join()`:
```cpp
bool DeviceInterface::getGpuMemInfoForFTDTM(QMap<QString,QString>& mapInfo) {
const QString c = readFileContent("/sys/kernel/debug/gc/total_mem");
if (c.isEmpty()) return false;
double value;
QString unit;
if (!parseMemInfo(c, value, unit)) return false;
const QString totalGb = convertToGbString(value, unit);
if (totalGb.isEmpty()) return false;
mapInfo.insert(kGraphicsMemory, totalGb);
return true;
}
QString DeviceInterface::getGpuInfoForFTDTM() {
static const QString info = []{
QMap<QString,QString> m;
if (!DeviceInterface{}.getGpuMemInfoForFTDTM(m))
return QString{};
QStringList lines;
for (auto it = m.constBegin(); it != m.constEnd(); ++it)
lines << QStringLiteral("%1: %2").arg(it.key(), it.value());
return lines.join(QLatin1Char('\n')) + QLatin1Char('\n');
}();
return info;
}
```
— now each helper does one job, the conversion is driven by a table instead of four branches, and your final formatting is a single `join()`.
</issue_to_address>
### Comment 2
<location> `deepin-devicemanager/src/Tool/commontools.cpp:269` </location>
<code_context>
QString CommonTools::preGenerateGpuInfo()
{
- static QString gpuInfo { "" };
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring preGenerateGpuInfo() by extracting caching and formatting logic into helper functions for clarity and maintainability.
Here’s one way to split retrieval/formatting out and collapse the nested logic in preGenerateGpuInfo():
```cpp
// helper to format a map into “key: value\n…”
static QString formatInfoMap(const QMap<QString,QString>& m) {
QString s;
for (auto it = m.begin(); it != m.end(); ++it)
s += it.key() + ": " + it.value() + "\n";
return s;
}
// caches/returns GPU‐base info
static QString getCachedGpuBaseInfo() {
static QString base;
if (base.isEmpty()) {
QMap<QString,QString> m;
if (!CommonTools::getGpuBaseInfo(m)) {
qCritical() << "Error: failed to get gpu base info!";
return {};
}
base = formatInfoMap(m);
}
return base;
}
// caches/returns GPU‐mem info
static QString getCachedGpuMemInfo() {
static QString mem;
if (mem.isEmpty()) {
QDBusInterface iface("org.deepin.DeviceInfo",
"/org/deepin/DeviceInfo",
"org.deepin.DeviceInfo",
QDBusConnection::systemBus());
if (!iface.isValid()) {
qCritical() << "Error: GPU mem DBus interface invalid";
} else {
auto reply = iface.call("getGpuInfoForFTDTM");
if (reply.isValid()) mem = reply.value();
else qCritical() << "Error: failed to call getGpuInfoForFTDTM";
}
}
return mem;
}
// simplified preGenerateGpuInfo()
QString CommonTools::preGenerateGpuInfo() {
auto base = getCachedGpuBaseInfo();
if (base.isEmpty()) return {}; // already logged
return base + getCachedGpuMemInfo();
}
```
– Moves string‐building into formatInfoMap
– Moves each cache into its own function
– Makes preGenerateGpuInfo() a straight-line composition of those two calls
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return result == Authority::Yes; | ||
| } | ||
|
|
||
| bool DeviceInterface::getGpuMemInfoForFTDTM(QMap<QString, QString> &mapInfo) |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the method by extracting helpers, using a unit conversion map, and simplifying formatting with join().
Here are a few low-risk refactorings you can apply to keep the exact same behaviour but break that monster method into bite-sized helpers, remove the deep if/else, and collapse your formatting loop into a single join().
- pull the file-read and empty-check out into its own helper:
static QString readFileContent(const QString& path) {
QFile f(path);
if (!f.open(QIODevice::ReadOnly|QIODevice::Text)) {
qCritical() << "Error opening" << path << ":" << f.errorString();
return {};
}
return QString::fromUtf8(f.readAll()).trimmed();
}- pull the regex match into a helper + make the regex
static constso it isn’t reallocated every call:
namespace {
static const QRegularExpression kMemRx{
R"((\d+(?:\.\d+)?)\s*\(?(MB|GB|KB|B)\)?)",
QRegularExpression::CaseInsensitiveOption
};
}
bool parseMemInfo(const QString& content, double& outValue, QString& outUnit) {
auto m = kMemRx.match(content);
if (!m.hasMatch()) {
qCritical() << "Error: no memory info in:" << content;
return false;
}
outValue = m.captured(1).toDouble();
outUnit = m.captured(2).toUpper();
return true;
}- replace the big
if/elsechain with a simple unit→factor map:
static const QHash<QString,double> kUnitToGb {
{ "B", 1.0/(1024*1024*1024) },
{ "KB", 1.0/(1024*1024) },
{ "MB", 1.0/1024.0 },
{ "GB", 1.0 }
};
QString convertToGbString(double value, const QString& unit) {
double factor = kUnitToGb.value(unit, 0);
if (factor == 0) {
qCritical() << "Unknown unit:" << unit;
return {};
}
return QString::number(value * factor, 'f', 2) + "GB";
}- re-write
getGpuMemInfoForFTDTM()as a simple orchestrator, and replace your manual loop ingetGpuInfoForFTDTM()with ajoin():
bool DeviceInterface::getGpuMemInfoForFTDTM(QMap<QString,QString>& mapInfo) {
const QString c = readFileContent("/sys/kernel/debug/gc/total_mem");
if (c.isEmpty()) return false;
double value;
QString unit;
if (!parseMemInfo(c, value, unit)) return false;
const QString totalGb = convertToGbString(value, unit);
if (totalGb.isEmpty()) return false;
mapInfo.insert(kGraphicsMemory, totalGb);
return true;
}
QString DeviceInterface::getGpuInfoForFTDTM() {
static const QString info = []{
QMap<QString,QString> m;
if (!DeviceInterface{}.getGpuMemInfoForFTDTM(m))
return QString{};
QStringList lines;
for (auto it = m.constBegin(); it != m.constEnd(); ++it)
lines << QStringLiteral("%1: %2").arg(it.key(), it.value());
return lines.join(QLatin1Char('\n')) + QLatin1Char('\n');
}();
return info;
}— now each helper does one job, the conversion is driven by a table instead of four branches, and your final formatting is a single join().
| return cmd; | ||
| } | ||
|
|
||
| QString CommonTools::preGenerateGpuInfo() |
There was a problem hiding this comment.
issue (complexity): Consider refactoring preGenerateGpuInfo() by extracting caching and formatting logic into helper functions for clarity and maintainability.
Here’s one way to split retrieval/formatting out and collapse the nested logic in preGenerateGpuInfo():
// helper to format a map into “key: value\n…”
static QString formatInfoMap(const QMap<QString,QString>& m) {
QString s;
for (auto it = m.begin(); it != m.end(); ++it)
s += it.key() + ": " + it.value() + "\n";
return s;
}
// caches/returns GPU‐base info
static QString getCachedGpuBaseInfo() {
static QString base;
if (base.isEmpty()) {
QMap<QString,QString> m;
if (!CommonTools::getGpuBaseInfo(m)) {
qCritical() << "Error: failed to get gpu base info!";
return {};
}
base = formatInfoMap(m);
}
return base;
}
// caches/returns GPU‐mem info
static QString getCachedGpuMemInfo() {
static QString mem;
if (mem.isEmpty()) {
QDBusInterface iface("org.deepin.DeviceInfo",
"/org/deepin/DeviceInfo",
"org.deepin.DeviceInfo",
QDBusConnection::systemBus());
if (!iface.isValid()) {
qCritical() << "Error: GPU mem DBus interface invalid";
} else {
auto reply = iface.call("getGpuInfoForFTDTM");
if (reply.isValid()) mem = reply.value();
else qCritical() << "Error: failed to call getGpuInfoForFTDTM";
}
}
return mem;
}
// simplified preGenerateGpuInfo()
QString CommonTools::preGenerateGpuInfo() {
auto base = getCachedGpuBaseInfo();
if (base.isEmpty()) return {}; // already logged
return base + getCachedGpuMemInfo();
}– Moves string‐building into formatInfoMap
– Moves each cache into its own function
– Makes preGenerateGpuInfo() a straight-line composition of those two calls
max-lvs
approved these changes
Oct 16, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs 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 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
deepin-bot
bot
merged commit ee4e916
into
linuxdeepin:develop/eagle
16 of 18 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
-- Adjust code logic to show gpu mem.
Log: fix issue
Bug: https://pms.uniontech.com/bug-view-336355.html
Summary by Sourcery
Fix GPU memory display by implementing debugfs reading and parsing, refactor related GPU info retrieval methods, and clean up outdated command-based logic.
Bug Fixes:
Enhancements: