Revert "feat: Introduce pzip high-performance parallel compression for ARM platform"#386
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR reverts the earlier introduction of the ARM-only pzip high‑performance compression plugin, restoring the previous libarchive/libzip behavior and heuristics, and removes all build, install, and runtime integration of pzip and its clipzip plugin. Sequence diagram for ARM slotCompress libarchive selection heuristicsequenceDiagram
actor User
participant MainWindow
participant CompressionEngine
participant LibarchivePlugin
participant LibzipPlugin
User ->> MainWindow: requestCompression(val)
MainWindow ->> MainWindow: slotCompress(val)
MainWindow ->> MainWindow: compute maxFileSize_ and m_stCompressParameter.qSize
alt ARM_aarch64
MainWindow ->> MainWindow: evaluate conditions
alt maxFileSize_ > 50MB
MainWindow ->> MainWindow: bUseLibarchive = true
else qSize > 200MB
MainWindow ->> MainWindow: bUseLibarchive = true
else qSize > 100MB and maxFileSize_ > 10MB
MainWindow ->> MainWindow: bUseLibarchive = true
else proportion > 0.6
MainWindow ->> MainWindow: bUseLibarchive = (maxFileSize_ / qSize) > 0.6
end
else Non_ARM
MainWindow ->> MainWindow: bUseLibarchive = false
end
alt bUseLibarchive == true
MainWindow ->> CompressionEngine: startCompression(LibarchivePlugin)
CompressionEngine ->> LibarchivePlugin: compressFiles()
else bUseLibarchive == false
MainWindow ->> CompressionEngine: startCompression(LibzipPlugin)
CompressionEngine ->> LibzipPlugin: compressFiles()
end
Class diagram for compression plugin architecture after pzip removalclassDiagram
class MainWindow {
- bool bUseLibarchive
- qint64 maxFileSize_
- CompressParameter m_stCompressParameter
+ void slotCompress(QVariant val)
}
class UiTools {
+ static ReadOnlyArchiveInterface* createInterface(QString fileName, bool bWrite)
}
class Plugin {
+ MetaData metaData()
}
class ReadOnlyArchiveInterface
class LibzipPlugin
class LibminizipPlugin
class LibarchivePlugin
class PzipPlugin_removed
class ClipzipPlugin_removed
MainWindow --> UiTools : uses
MainWindow --> ReadOnlyArchiveInterface : configures_compression
UiTools --> ReadOnlyArchiveInterface : createInterface
UiTools --> Plugin : iterates_offers
Plugin <|-- LibzipPlugin
Plugin <|-- LibminizipPlugin
Plugin <|-- LibarchivePlugin
Plugin <|-- PzipPlugin_removed
Plugin <|-- ClipzipPlugin_removed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the ARM-specific compression selection logic, guard the
maxFileSize_ / m_stCompressParameter.qSizecomputation against a possibleqSize == 0to avoid division-by-zero in edge cases (e.g., empty selections). - The series of size/threshold checks in
slotCompressfor__aarch64__would be easier to maintain and reason about if extracted into a small helper function (e.g.,shouldUseLibarchiveForArm(...)) with clearly named parameters and possibly shared between call sites if used elsewhere.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the ARM-specific compression selection logic, guard the `maxFileSize_ / m_stCompressParameter.qSize` computation against a possible `qSize == 0` to avoid division-by-zero in edge cases (e.g., empty selections).
- The series of size/threshold checks in `slotCompress` for `__aarch64__` would be easier to maintain and reason about if extracted into a small helper function (e.g., `shouldUseLibarchiveForArm(...)`) with clearly named parameters and possibly shared between call sites if used elsewhere.
## Individual Comments
### Comment 1
<location path="src/source/mainwindow.cpp" line_range="1093-1094" />
<code_context>
+ bUseLibarchive = true;
+ }
+ // 大文件占比超过60%使用libarchive
+ else {
+ double maxFileSizeProportion = static_cast<double>(maxFileSize_) / static_cast<double>(m_stCompressParameter.qSize);
+ bUseLibarchive = maxFileSizeProportion > 0.6;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against potential division by zero when computing maxFileSizeProportion.
If `m_stCompressParameter.qSize` can be 0 (e.g., empty selection or placeholder jobs), this will divide by zero. Consider guarding against `qSize == 0` (e.g., early-return or skipping this branch) before computing `maxFileSizeProportion`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| else { | ||
| double maxFileSizeProportion = static_cast<double>(maxFileSize_) / static_cast<double>(m_stCompressParameter.qSize); |
There was a problem hiding this comment.
issue (bug_risk): Guard against potential division by zero when computing maxFileSizeProportion.
If m_stCompressParameter.qSize can be 0 (e.g., empty selection or placeholder jobs), this will divide by zero. Consider guarding against qSize == 0 (e.g., early-return or skipping this branch) before computing maxFileSizeProportion.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, 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 |
|
/merge |
Reverts #372
Summary by Sourcery
Revert the previously introduced pzip-based high-performance ZIP compression support and restore the prior compression behavior, particularly on ARM platforms.
Enhancements:
Build: