Seek and destroy main memory leak #1444
Merged
Conversation
This file contains 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
rafaellehmkuhl
force-pushed
the
seek-and-destroy-memory-leak
branch
from
df5de03 to
62c6ed0
Compare
The `v-tooltip` attribute-based component has a bug: it is never eliminated once instantiated, and as the alerts list is regenerated for every new alert, the tooltip component was accumulating in an exponential way, leading to a crash in the whole application. For the moment a simple `title` attribute is used, but it's not an ideal solution, as it takes some hovering time to be activated. A better solution involves creating/importing a new tooltip component that doesn't have the same bug or to expand the alert space to fit the text during hovering.
rafaellehmkuhl
force-pushed
the
seek-and-destroy-memory-leak
branch
from
f8965d5 to
e8dee51
Compare
rafaellehmkuhl
changed the title
rafaellehmkuhl
changed the title
ArturoManzoli
approved these changes
Nov 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I performed about a week of testing with Cockpit to learn more about its performance issues.
A detailed report can be found here, but I will make a resume below.
The main leak
I could find that we indeed have a big memory leak that causes the application to become slow after about 20 minutes and to crash after around an hour (depending on the computer specs). After a lot of tests I could determine that this leak is caused by a
v-tooltipcomponent that is used in the expanded list of theAlerter.vuewidget. This component is instantiated for every alert in the list, and is never garbage-collected, so for every new alert added to the list, a series of zombie components are created and left there forever. This PR replaces that component, effectively removing the main memory leak.Before:
After:
Other leaks
I could also find that we have smaller memory leaks (with unknown sources) that grow very slowly, without interfering in most (99%?) user sessions, and that we have a concentrated memory leak when the joystick configuration page is open.
Future work
This PR only fixes the main memory leak (from the
Alerter.vuewidget), but a following PR should be done fixing the problem in the joystick configuration page as well, as it affects every user that opens that view.We should also eventually have a better solution than the
titleattribute to replace the buggy tooltip, as the attribute does not work on mobile and takes a somewhat long hovering time to shown, degrading the UX.Result
With this PR, the user can now effectively run Cockpit for hours without it becoming slow or crashing.