[MGNLUI-4004] Asynchronous actions can consume a lot of memory Created: 02/Sep/16  Updated: 09/Feb/17  Resolved: 15/Sep/16

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: 5.3.9, 5.4.x
Fix Version/s: 5.3.17, 5.4.10, 5.5

Type: Bug Priority: Critical
Reporter: Philipp Bärfuss Assignee: Federico Grilli
Resolution: Fixed Votes: 2
Labels: None
Remaining Estimate: 0d
Time Spent: 2d 3.5h
Original Estimate: Not Specified

Attachments: PNG File 5.4.x-enclosing-instance-retained.png     PNG File 5.4.x-scheduler-keeps-trigger-listeners-after-done.png     PNG File clear trigger listeners.png     PNG File current impl retained size.png     PNG File fixed impl retained size.png     PNG File incoming res + path to GC.png     PNG File no trigger listener removal.png    
Issue Links:
relation
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Release notes required:
Yes
Date of First Response:
Sprint: Basel 61
Story Points: 5

 Description   

The inner class AbstractCommandAction.CommandActionTriggerListener is not declared static and hence each instance is bound to the parent action object. This action object has references to UI related objects. This way the asynchronous tasks in the scheduler use much more heap than needed which makes the GCs job unnecessary hard.

We have seen a system where this added up to 1Gigabite of heap.

If I am right then adding the static modifier to the inner class would be enough.



 Comments   
Comment by Federico Grilli [ 05/Sep/16 ]

If you declare a member class that does not require access to an enclosing instance, always put the static modifier in its declaration, making it a static rather than a nonstatic member class. If you omit this modifier, each instance will have an extraneous reference to its enclosing instance. Storing this reference costs time and space, and can result in the enclosing instance being retained when it would otherwise be eligible for garbage collection (Item 6)

– Joshua Bloch, Effective Java

I know, I'm very fond of this piece of wisdom

Comment by Federico Grilli [ 12/Sep/16 ]

From my first investigations and debugging, making the inner listener class static certainly helps to reduce the size of the object in memory (from ~6kB to less than 1kB) as you can see from the attached profiler screenshot. Still those objects apparently won't be freed by GC because they are kept internally by the Quartz scheduler, see
.
I also looked into QuartzScheduler source code and apparently they don't get removed after a job is completed and the trigger completed fired. I'll keep on investigating whether there is a possibility Quartz removes those listeners under certain conditions.

UPDATE
------------
So apparently QuartzScheduler does not get rid of TriggerListerer(s) itself. So we're going to do it ourselves precisely in the AbstractCommandAction.CommandActionTriggerListener#triggerComplete method where we're sure we only delete the current listener once it's done with its job. This way, heap usage should no longer grow due to listener classes never being removed from QuartzScheduler sole instance. The latter is stored in a field of SchedulerModule which is a singleton and thus lives in memory for the whole application lifetime. Below a couple of screenshots from profiler show before the removal fix and after.

Before:

Each time an activation is launched from the UI, the CommandTriggerListener class is kept in memory even when the job has completed (in this case two activations were started)

After:

Comment by Federico Grilli [ 14/Sep/16 ]

Just checked 5.4.x branch which is the same as master or 5.5 in this case. Same issue as in 5.3.x. I would therefore apply the same fix to those branches before the problem shows up in some production environment.

Generated at Mon Feb 12 09:12:18 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.