-
Notifications
You must be signed in to change notification settings - Fork 40
Fix: [gpu-info] The gpu mem not show. #543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -268,42 +268,40 @@ QString CommonTools::getGpuInfoCommandFromDConfig() | |
|
|
||
| QString CommonTools::preGenerateGpuInfo() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| { | ||
| static QString gpuInfo { "" }; | ||
|
|
||
| if (gpuInfo.isEmpty()) { | ||
| QStringList arguments; | ||
| QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); | ||
| QString display = env.value("DISPLAY"); | ||
| QString xauthority = env.value("XAUTHORITY"); | ||
| if (display.isEmpty() || xauthority.isEmpty()) { | ||
| qCritical() << "DISPLAY or XAUTHORITY is not set!"; | ||
| } else { | ||
| arguments << display << xauthority; | ||
| static QString gpuBaseInfo { "" }; | ||
| static QString gpuMemInfo { "" }; | ||
|
|
||
| if (gpuBaseInfo.isEmpty()) { | ||
| QMap<QString, QString> mapInfo; | ||
| if (getGpuBaseInfo(mapInfo)) { | ||
| for (auto it = mapInfo.begin(); it != mapInfo.end(); ++it) { | ||
| QString tmpInfo = it.key() + ": " + it.value() + "\n"; | ||
| gpuBaseInfo.append(tmpInfo); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (gpuBaseInfo.isEmpty()) { | ||
| qCritical() << "Error: failed to get gpu base info!"; | ||
| return ""; | ||
| } | ||
|
|
||
| if (gpuMemInfo.isEmpty()) { | ||
| QDBusInterface iface("org.deepin.DeviceInfo", | ||
| "/org/deepin/DeviceInfo", | ||
| "org.deepin.DeviceInfo", | ||
| QDBusConnection::systemBus()); | ||
| if (iface.isValid()) { | ||
| QDBusReply<QString> replyList = iface.call("getGpuInfoByCustom", arguments, gpuInfo); | ||
| QDBusReply<QString> replyList = iface.call("getGpuInfoForFTDTM"); | ||
| if (replyList.isValid()) { | ||
| gpuInfo = replyList.value(); | ||
| gpuMemInfo = replyList.value(); | ||
| } else { | ||
| qCritical() << "Error: failed to call dbus to get gpu memery info! "; | ||
| } | ||
| } | ||
|
|
||
| QMap<QString, QString> mapInfo; | ||
| if (getGpuBaseInfo(mapInfo)) { | ||
| for (auto it = mapInfo.begin(); it != mapInfo.end(); ++it) { | ||
| QString tmpInfo = it.key() + ": " + it.value() + "\n"; | ||
| gpuInfo.append(tmpInfo); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return gpuInfo; | ||
| return (gpuBaseInfo + gpuMemInfo); | ||
| } | ||
|
|
||
| bool CommonTools::getGpuBaseInfo(QMap<QString, QString> &mapInfo) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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().static constso it isn’t reallocated every call:if/elsechain with a simple unit→factor map:getGpuMemInfoForFTDTM()as a simple orchestrator, and replace your manual loop ingetGpuInfoForFTDTM()with ajoin():— 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().