Skip to content
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 possible memory leak in CSS Classes of cells #2524

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

Marc-Spector
Copy link
Collaborator

@Marc-Spector Marc-Spector commented Jan 13, 2022

I connected the profiler to the client.
As you can see from the graph, memory consumption increases dramatically during each minute, until the garbage collector starts its work. It looks like a memory leak.
After analyzing the code, I came to the conclusion that the styleClasses object is not a hash collection, which is why styleClasses contains many identical class names, which in turn triggers many monotonous actions (unexplained CPU spikes)

Before fix:
image

After fix:
image

@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #2524 (1dacaab) into develop (14d40ba) will increase coverage by 0.26%.
The diff coverage is 28.57%.

@@              Coverage Diff              @@
##             develop    #2524      +/-   ##
=============================================
+ Coverage      63.99%   64.25%   +0.26%     
- Complexity      4424     4483      +59     
=============================================
  Files            478      478              
  Lines          19301    19454     +153     
  Branches        1120     1144      +24     
=============================================
+ Hits           12351    12500     +149     
- Misses          6326     6331       +5     
+ Partials         624      623       -1     
Impacted Files Coverage Δ
...va/com/faforever/client/fx/DualStringListCell.java 0.00% <0.00%> (ø)
...rc/main/java/com/faforever/client/fx/IconCell.java 61.53% <0.00%> (+2.71%) ⬆️
...ain/java/com/faforever/client/fx/NodeListCell.java 0.00% <0.00%> (ø)
...in/java/com/faforever/client/fx/NodeTableCell.java 0.00% <0.00%> (ø)
.../main/java/com/faforever/client/fx/StringCell.java 85.71% <ø> (-4.29%) ⬇️
...main/java/com/faforever/client/fx/DecimalCell.java 90.90% <100.00%> (-0.76%) ⬇️
...n/java/com/faforever/client/fx/StringListCell.java 47.61% <100.00%> (+2.16%) ⬆️
...ever/client/player/PlayerInfoWindowController.java 72.40% <0.00%> (-1.83%) ⬇️
...om/faforever/client/chat/ChannelTabController.java 81.71% <0.00%> (+0.38%) ⬆️
...ever/client/leaderboard/LeaderboardController.java 89.68% <0.00%> (+1.92%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14d40ba...1dacaab. Read the comment docs.

@Sheikah45 Sheikah45 changed the title Fix possible memory leak Fix possible memory leak in CSS Classes of cells Jan 21, 2022
@Sheikah45 Sheikah45 merged commit ab30d2e into develop Jan 21, 2022
@Sheikah45 Sheikah45 deleted the bugfix/fix-memory-leak branch January 21, 2022 00:12
fultonm pushed a commit to fultonm/downlords-faf-client that referenced this pull request Jan 23, 2022
mrchris2000 pushed a commit to mrchris2000/downlords-faf-client that referenced this pull request Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants