-
Notifications
You must be signed in to change notification settings - Fork 55
Fix few memory leaks #1289
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
Fix few memory leaks #1289
Conversation
The `MultiEffect` component was identified as a potential source of memory leaks and performance issues when window blur effects are enabled. This change replaces `MultiEffect` with the more lightweight and efficient `FastBlur` component. This should reduce memory consumption and improve performance, addressing the user's report of a memory leak.
This commit addresses two issues: 1. A potential memory leak and performance degradation when window blur effects are enabled. This is fixed by replacing the `MultiEffect` QML component with the more lightweight and efficient `FastBlur` in `InWindowBlur.qml`. 2. A potential freezing issue in the dde-dock, which was likely caused by an unreliable `QTimer::singleShot` used to handle screen changes. This is fixed by replacing the timer with a more robust `QMetaObject::invokeMethod` call with a `QueuedConnection` in `dockpanel.cpp`.
This commit addresses two issues: 1. A potential memory leak and performance degradation when window blur effects are enabled. This is fixed by replacing the `MultiEffect` QML component with the more lightweight and efficient `FastBlur` in `InWindowBlur.qml`. 2. A potential freezing issue in the dde-dock, which was likely caused by an unreliable `QTimer::singleShot` used to handle screen changes. This is fixed by replacing the timer with a more robust `QMetaObject::invokeMethod` call with a `QueuedConnection` in `dockpanel.cpp`.
This commit addresses several critical issues to improve the long-term stability and performance of the Deepin Desktop Environment shell and dock. 1. **Memory Leak in `dockdbusproxy.cpp`**: Fixed a memory leak where a `QTimer` was allocated without a parent, preventing it from being properly garbage collected. It is now correctly parented. 2. **Memory Leak in `rolegroupmodel.cpp`**: Fixed a memory leak caused by raw pointers to `QList<int>` in a `QHash`. A destructor has been added to `RoleGroupModel` to ensure this memory is deallocated. 3. **Dock Freezing in `dockpanel.cpp`**: Replaced an unreliable `QTimer::singleShot` with a more robust `QMetaObject::invokeMethod` using a `QueuedConnection`. This makes the handling of screen changes safer and should prevent the dock from freezing. 4. **Performance Issue in `InWindowBlur.qml`**: Replaced the resource-intensive `MultiEffect` component with the more lightweight and efficient `FastBlur`. This directly addresses performance degradation and potential memory leaks when window blur effects are enabled.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: csfercoci 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 |
|
Hi @csfercoci. Thanks for your PR. 😃 |
|
Hi @csfercoci. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
CLA Assistant Lite bot: You can retrigger this bot by commenting recheck in this Pull Request |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR resolves multiple memory leaks and stability issues by correctly parenting timers, adding destructors for dynamic allocations, replacing unreliable timer calls with queued invocations, and optimizing a QML blur effect component. Sequence diagram for improved dock screen change handlingsequenceDiagram
participant DockPanel
participant Window
DockPanel->>Window: Check m_dockScreen vs window()->screen()
alt m_dockScreen != window()->screen()
DockPanel->>DockPanel: QMetaObject::invokeMethod(..., Qt::QueuedConnection)
DockPanel->>Window: setScreen(m_dockScreen)
DockPanel->>DockPanel: onWindowGeometryChanged()
else
DockPanel->>DockPanel: onWindowGeometryChanged()
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Replace raw pointer-based data structures in RoleGroupModel with value-based containers (e.g. QVector) to eliminate manual memory management and qDeleteAll calls.
- Verify that FastBlur in InWindowBlur.qml fully matches the visual and performance characteristics of the original MultiEffect, potentially parameterizing key blur settings instead of hardcoding.
- In DockPanel, you could simplify the delayed screen update by using a parented QTimer::singleShot call or a named slot instead of QMetaObject::invokeMethod with a lambda to improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Replace raw pointer-based data structures in RoleGroupModel with value-based containers (e.g. QVector<int>) to eliminate manual memory management and qDeleteAll calls.
- Verify that FastBlur in InWindowBlur.qml fully matches the visual and performance characteristics of the original MultiEffect, potentially parameterizing key blur settings instead of hardcoding.
- In DockPanel, you could simplify the delayed screen update by using a parented QTimer::singleShot call or a named slot instead of QMetaObject::invokeMethod with a lambda to improve readability.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…m stability and performance of the Deepin Desktop Environment shell and dock. 1. **Refactor `RoleGroupModel` for Memory Safety**: Replaced raw pointer-based containers with value-based `QHash` and `QList`. This eliminates manual memory management (`new`/`delete`) and resolves a class of memory leaks. 2. **Fix `QTimer` Memory Leak in `dockdbusproxy.cpp`**: Ensured a `QTimer` is parented, allowing Qt's memory management to handle its destruction and preventing a leak. 3. **Improve Dock Stability in `dockpanel.cpp`**: Refactored the screen change handler to use a zero-delay `QTimer::singleShot`. This provides a safe, deferred execution for the screen update, improving readability and preventing potential freezes. 4. **Enhance Performance in `InWindowBlur.qml`**: Replaced the resource-intensive `MultiEffect` with the more lightweight and efficient `FastBlur` to address performance degradation and potential memory leaks when window blur effects are enabled.
…m stability and performance of the Deepin Desktop Environment shell and dock. 1. **Refactor `RoleGroupModel` for Memory Safety**: Replaced raw pointer-based containers with value-based `QHash` and `QList`. This eliminates manual memory management (`new`/`delete`) and resolves a class of memory leaks. 2. **Fix `QTimer` Memory Leak in `dockdbusproxy.cpp`**: Ensured a `QTimer` is parented, allowing Qt's memory management to handle its destruction and preventing a leak. 3. **Improve Dock Stability in `dockpanel.cpp`**: Refactored the screen change handler to use a zero-delay `QTimer::singleShot`. This provides a safe, deferred execution for the screen update, improving readability and preventing potential freezes. 4. **Enhance Performance in `InWindowBlur.qml`**: Replaced the resource-intensive `MultiEffect` with the more lightweight and efficient `FastBlur` to address performance degradation and potential memory leaks when window blur effects are enabled.
| } | ||
|
|
||
| void RoleGroupModel::adjustMap(int base, int offset) | ||
| void RoleGroup_model::adjustMap(int base, int offset) |
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.
是不是误操作?
Fix: Resolve memory leaks and improve stability in dde-shell
This commit addresses several critical issues to improve the long-term stability and performance of the Deepin Desktop Environment shell and dock.
Memory Leak in
dockdbusproxy.cpp: Fixed a memory leak where aQTimerwas allocated without a parent, preventing it from being properly garbage collected. It is now correctly parented.Memory Leak in
rolegroupmodel.cpp: Fixed a memory leak caused by raw pointers toQList<int>in aQHash. A destructor has been added toRoleGroupModelto ensure this memory is deallocated.Dock Freezing in
dockpanel.cpp: Replaced an unreliableQTimer::singleShotwith a more robustQMetaObject::invokeMethodusing aQueuedConnection. This makes the handling of screen changes safer and should prevent the dock from freezing.Performance Issue in
InWindowBlur.qml: Replaced the resource-intensiveMultiEffectcomponent with the more lightweight and efficientFastBlur. This directly addresses performance degradation and potential memory leaks when window blur effects are enabled.Summary by Sourcery
Resolve several memory leaks and improve stability and performance in the dde-shell and dock components
Bug Fixes:
Enhancements: