compression level in clipzipplugin for improved performance#343
Merged
deepin-bot[bot] merged 3 commits intolinuxdeepin:develop/snipefrom Jan 15, 2026
Merged
Conversation
- Updated CMake configuration to conditionally enable pzip and clipzipplugin for ARM architecture with Qt6. - Improved signal-slot connections in clipzipplugin for compatibility with different Qt versions. - Added compression level options and refined process completion handling. Log: Enhance ARM + Qt6 support for clipzipplugin
- Added logic to skip the pzip plugin when reading ZIP files, as it is write-only. - Introduced debug messages to indicate when the pzip plugin is being removed from the plugin selection process. Log: Improve handling of pzip plugin for ZIP file reading
Reviewer's GuideUpdates the CliPzip (pzip) integration to favor fast compression settings, modernizes QProcess signal handling for Qt5/Qt6, limits the pzip-based plugin to ARM + Qt6 builds, and ensures the pzip plugin is only used for writing ZIPs (not reading) with clearer runtime logging. Sequence diagram for skipping pzip plugin when reading ZIPsequenceDiagram
actor User
participant UiTools
participant PluginManager
participant PzipPlugin
participant LibzipPlugin
User->>UiTools: createInterface(fileName, bWrite=false)
UiTools->>UiTools: detect mimeType = application/zip
UiTools->>UiTools: set removePzipFlag = true
UiTools->>PluginManager: query offers for fileName
PluginManager-->>UiTools: offers [PzipPlugin, LibzipPlugin, ...]
loop iterate offers
UiTools->>PzipPlugin: check metaData().name()
UiTools->>UiTools: removePzipFlag && name contains pzip
UiTools-->>PzipPlugin: skip plugin
UiTools->>LibzipPlugin: check metaData().name()
UiTools->>UiTools: removePzipFlag && name does not contain pzip
UiTools-->>LibzipPlugin: select plugin
end
UiTools-->>User: ReadOnlyArchiveInterface using LibzipPlugin
Class diagram for updated CliPzipPlugin integrationclassDiagram
class ReadWriteArchiveInterface
class KPtyProcess
class QTimer
class CliPzipPluginFactory {
+CliPzipPluginFactory()
+~CliPzipPluginFactory()
+create()
}
class CliPzipPlugin {
+CliPzipPlugin()
+~CliPzipPlugin()
+PluginFinishType extractFiles(files, options)
+PluginFinishType addFiles(files, options)
-void readStdout(bool handleAll)
-void processFinished(int exitCode, QProcess::ExitStatus exitStatus)
-void processFinished(int exitCode)
-void deleteProcess()
-KPtyProcess* m_process
-QTimer* m_timer
}
CliPzipPluginFactory --> CliPzipPlugin
CliPzipPlugin --|> ReadWriteArchiveInterface
CliPzipPlugin o-- KPtyProcess : manages
CliPzipPlugin o-- QTimer : manages
class QProcess {
+void start()
+ExitStatus exitStatus()
+signal finished(int exitCode, ExitStatus exitStatus)
+signal finished(int exitCode)
}
CliPzipPlugin ..> QProcess : uses for pzip
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
- Updated clipzipplugin to enforce a compression level of 1, prioritizing speed over user-defined settings. - Removed the option to specify compression levels from the GUI, simplifying the configuration for optimal performance. Log: Enhance performance by standardizing compression level in clipzipplugin bug: https://pms.uniontech.com/bug-view-346679.html
dengzhongyuan365-dev
force-pushed
the
pzip-cpp
branch
from
16083ee to
4a1f837
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The plugin skipping logic relies on
plugin->metaData().name().contains("pzip", Qt::CaseInsensitive), which is a bit fragile; consider matching on a more stable identifier (e.g., plugin id or exact name) to avoid accidentally skipping unrelated plugins with similar names. - The
ARM + Qt6condition is duplicated in both the top-level and3rdpartyCMakeLists; it may be worth centralizing this into a single option/variable or helper macro to keep the enablement logic consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The plugin skipping logic relies on `plugin->metaData().name().contains("pzip", Qt::CaseInsensitive)`, which is a bit fragile; consider matching on a more stable identifier (e.g., plugin id or exact name) to avoid accidentally skipping unrelated plugins with similar names.
- The `ARM + Qt6` condition is duplicated in both the top-level and `3rdparty` CMakeLists; it may be worth centralizing this into a single option/variable or helper macro to keep the enablement logic consistent and easier to maintain.
## Individual Comments
### Comment 1
<location> `3rdparty/clipzipplugin/clipzipplugin.cpp:198-202` </location>
<code_context>
+ arguments << "-l" << "1";
+ Q_UNUSED(options.iCompressionLevel); // 忽略 GUI 传递的压缩级别
+
+ // 线程数：只有大于1时才指定，否则让 pzip 自动使用全部 CPU 核心
+ if (options.iCPUTheadNum > 1) {
+ arguments << "-c" << QString::number(options.iCPUTheadNum);
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The new thread-count logic changes behavior for `iCPUTheadNum == 1` and might not match the comment or user expectations.
Previously, any `iCPUTheadNum > 0` produced a `-c` option. Now, `iCPUTheadNum == 1` sends no `-c`, which (per the comment) makes pzip use all cores. So a user choosing “1 thread” in the GUI effectively gets “all cores,” not a single-threaded run. If this is intentional, consider removing the “1 thread” option from the GUI or mapping `1` to pzip’s single-thread syntax (if available). Otherwise, keep the old behavior for `== 1`.
```suggestion
// 线程数：GUI 中设置的大于 0 的线程数都传递给 pzip
// iCPUTheadNum <= 0 时不传 -c 参数，pzip 默认使用全部核心
if (options.iCPUTheadNum > 0) {
arguments << "-c" << QString::number(options.iCPUTheadNum);
}
```
</issue_to_address>
### Comment 2
<location> `src/source/common/uitools.cpp:289-296` </location>
<code_context>
}
+ // pzip 插件只用于压缩，不用于读取/解压
+ bool removePzipFlag = (!bWrite) && (mimeType.name() == QString("application/zip"));
+ if (removePzipFlag) {
+ qDebug() << "Setting flag to remove pzip plugin for reading zip (pzip is write-only)";
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Skipping the plugin based on `mimeType` string equality and plugin name substring is somewhat brittle.
This relies on two brittle conditions: `mimeType.name() == "application/zip"` and `plugin->metaData().name().contains("pzip", Qt::CaseInsensitive)`. It may fail if other ZIP MIME types are introduced (e.g. `application/x-zip`) or if the plugin name changes or is localized. Prefer a more stable identifier (e.g. plugin ID or a dedicated metadata flag for write-only pzip), and consider broadening the ZIP MIME check if other ZIP types are supported elsewhere.
Suggested implementation:
```cpp
// pzip 插件只用于压缩，不用于读取/解压
//
// 使用更稳健的 MIME 类型检测方式：通过 inherits() 支持多种 ZIP MIME 类型，
// 避免仅依赖于单一字符串 "application/zip"。
const bool isZipMimeType =
mimeType.inherits(QStringLiteral("application/zip")) ||
mimeType.inherits(QStringLiteral("application/x-zip")) ||
mimeType.inherits(QStringLiteral("application/x-zip-compressed"));
bool removePzipFlag = (!bWrite) && isZipMimeType;
if (removePzipFlag) {
qDebug() << "Setting flag to remove pzip plugin for reading zip (pzip is write-only)"
<< "for MIME type:" << mimeType.name();
}
```
```cpp
// 读取 ZIP 时跳过 pzip 插件（pzip 只用于压缩）
//
// 优先使用稳定的插件标识符（pluginId），避免依赖名字的子串匹配。
const auto metaData = plugin->metaData();
const QString pluginId = metaData.pluginId();
const QString pluginName = metaData.name();
const bool isPzipPlugin =
(!pluginId.isEmpty() &&
(pluginId.compare(QStringLiteral("pzip"), Qt::CaseInsensitive) == 0 ||
pluginId.compare(QStringLiteral("org.example.pzip"), Qt::CaseInsensitive) == 0)) ||
(pluginId.isEmpty() &&
pluginName.compare(QStringLiteral("pzip"), Qt::CaseInsensitive) == 0);
if (removePzipFlag && isPzipPlugin) {
```
Depending on how plugin metadata is defined in your project:
1. Ensure that the pzip plugin has a stable `pluginId` set in its metadata (e.g. `"pzip"` or `"org.example.pzip"`), and adjust the string(s) in the `pluginId.compare(...)` calls above to match the actual ID used in your plugin JSON/metadata.
2. If your plugin system does not expose `pluginId()` on `metaData`, replace `metaData.pluginId()` with the appropriate API used in your codebase for a stable identifier (for example `metaData.pluginId()` vs `metaData.id()` vs a custom property).
3. If other ZIP MIME types are supported elsewhere in the codebase, extend the `isZipMimeType` expression with those types so behavior is consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review这段代码主要进行了以下几方面的修改：
以下是对这些修改的详细审查和改进建议： 1. CMakeLists.txt (3rdparty 和 根目录)审查意见：
改进建议：
2. clipzipplugin.cpp 和 clipzipplugin.h审查意见：
改进建议：
3. kerfuffle_clipzip.json审查意见：
改进建议：
4. uitools.cpp审查意见：
改进建议：
总结这段代码的修改整体上是高质量且安全的，主要目的是针对特定硬件（ARM）优化压缩性能，并适配 Qt6。主要的改进空间在于：
|
lzwind
approved these changes
Jan 15, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengzhongyuan365-dev, lzwind 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 |
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
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.
feat: Enhance ZIP plugin handling in UiTools
Log: Improve handling of pzip plugin for ZIP file reading
Summary by Sourcery
Optimize pzip-based ZIP compression and limit its usage to supported environments while ensuring it is not used for reading ZIP archives.
New Features:
Bug Fixes:
Enhancements:
Build: