Improve memory footprint

[sghpjuikit]

Umbrella for long-term effort of bringing down memory consumption observed by an OS. Hence this involves

• fixing memory leaks
• optimizing application
• tweaking gc, see Configure gc #46

The goal is:

• faster startup/performance
• no need to ever call System.gc(), not by user nor programmatically
• recovering from memory peaks
• long-running sessions (no leaks)

Testing:
The use case I'll be running with is DirView loading many images, because it causes quick and high memory peak that our application must be able to get through back to normal.

[sghpjuikit]

I have profiled the application and discovered/confirmed several problems.

1 image grid component has a massive leak. After taking all precautions and confirming (with heap dump) no references to the image data on our code, the problem still persists due to (leaky?) references in deep internals of JavaFX code. Nothing helps, setting ImageView.image to null, clearing all ImageView references... There are some hashmaps containing the images in NGRegion (which is Node.peer). After hours of trying the one solution that worked for me was to use reflection on ImageView.image.platformImage.pixelBuffer/pixelaccessor and set them to null. Consequent call to gc immediatelly disposes all image data, confirmed both in heap dump and OS process manager. Fixing this will involve

• javafx hack to dispose image manually
• gc tuning to promptly return memory to OS, so far I still needed to run gc() manually to get the memory back immediatelly
• improving grid component to dispose images while scrolling to prevent large amount of loaded images, this will most probably be done by time eviction.

2 window icons. We provide 7 size version, while only 1-2 may be necessary. These images also get computed each time a new window is created. I have put this into a lazy property and consider it fixed, but we may remove some icon versions in the future.

3 For autocomplete during tagging, all values of all String representable Metadata fields are grouped in a Map<Metadata.Field,Set<String>>. This involves date fields, comment, tags and other fields that contain long or always-unique strings. I have reduced the autocompletable fields to handpicked few. This does helps memory usage a lot for bigger library.

4 Kotlin intrinsics.checkNonNull. I haven't done rigorous profiling, but i did observe kotlin generated nullchecks to hog a lot of the cpu, in fact once I observed a method with total CPU running time 20s to waste almost all its computation time in the null check. I have already noticed this issue 2 years ago when I was profiling. I'm experimenting with -Xno-call-assertions, -Xno-param-assertions. This may make developer's life slightly less convenient if null gets through somewhere, so we may use this only in production. Right now I'm enabling this to see what will happen.

[sghpjuikit]

1 GridView image disposing has been improved in 1e7b601. Ultimately we do rely on reflection to get rid of the underlying image data reference, but I saw no other way to do this. Image disposing (in GridFileThumbCell at least) definitely works now. The memory is not released immediately, but that is left up to the gc tuning.

4 The observed overhead of Kotlin intrinsics.checkNonNull may be exacerbated by profiling.

I'll continue to profile and improve the memory footprint of the application.

We can also explore CompressedOops option of 64-bit JVM, I remember 32-bit version consuming literally half of the memory in the past, although as far as I know this option is already enabled by default.

[xeruf]

Regarding 4: I have also observed that while profiling an application of mine, which used recursion and a lot of small methods and thus wasted quite some cycles on this checkNonNull call. Especially since almost all projects I work on are pure Kotlin, this is mostly a waste. It only carries advantages if you interact with Java code.

[sghpjuikit]

Yeah.

Another overly memory-demanding objects I have identified are:

• StyleManager and its caches. According to VisualVM this takes more memory than Metadata of my entire audio library! I have already fixed issue where skin was applied multiple times, but I think I will be able to avoid applying the skin on popups and the like, possibly further reducing the footprint. I will also try to avoid excessive skin importing (Currently default skin imports skin that imports another skin, applied on top of default JavaFX skin).
• ContextMenus. Some context menus that user is unlikely to use more than very rarely are consuming alarming amount of memory (possibly because of icons). I will try to build the menu-items lazily.
• ZipFile in class loader. This is normally not a problem but it could (and should) be improved. Right now widget dependencies can not be removed while app is running, because the ClassLoader holds file handles. I will need to explicitly close the URLClassLoaders that load widgets, but first I need to figure out when is the soonest time in widget's lifecycle that this can be done.

[xeruf]

• You do know that you don't have to apply the default skin, right? Have you purged the skins of their superfluous copied properties?
• Aren't they all lazily-built? I thought we had that whole CoreMenus class for handling that?
  Could it be that the icon files are unnecessarily big?
• Where do you use ZipFiles?

[sghpjuikit]

• I'm not specifically applying the Modena skin, but it is probably applied internally. The purging is WIP.
• You are correct, but there are few hardcoded menus, mostly for table column management and these appear to be expensive
• ZipFile is used internally by URLClassLoader for jar dependencies. It needs to be explicitly closed to free those resources.

[sghpjuikit]

• I have cleaned up the skin css' a bit, fixed global variable definitions and made the default skin to only use one import. No idea about whether it helped. The property overriding should not cause problems and while skin cleanup continues to be WIP, I consider the potential css memory "problem" non-existent. I may try to merge default skin Flow into Main to remove the remaining import, no point bloating default skin, is there. Anyway... Fixed.
• I have tried to change skin application to only be used on roots of appliciation windows (no popups, etc), but there were always some problems (some popups not respecting the values, etc). Although skin application happens for every open window (including popups, tooltips, menus), this should not be a problem as besides popups multiple instances of these can not be open simultaneously anyway. I consider this optimized. Fixed.
• Making menu lazy appears to have had an effect - no more expensive menu items instances around. All menu items are now computed on demand, which is inefficient if same menu is opened multiple times but 1 the items are now removed when menu closes and they should be gced and 2 this is necessary to build the menu from current content anyway - better be consistent. So... Fixed.

Another point of interest coming from heap dump:

• StyleManager. PseudoClassState.class has 250k instances in my application! Each instance is 32 or 64 bytes, but with that many instances... Either we are doing something wrong (css imports, skin applications, etc.) or JavaFX has some styling implementation issues. These are all in ClassLoader.classes[StyleManager.class]. This single class is responsible for 50% memory retained by ClassLoader.classes!
  It is interesting that PseudoClassState instances have several optimization mechanisms, including cache, so this feels wrong.
  A lot of instances refer to cells in tables. This could also be because we set font across the application using setStyle("...") which may be very inefficient.

[xeruf]

A lot of instances refer to cells in tables. This could also be because we set font across the application using setStyle("...") which may be very inefficient.

Why do you not set the font in css?
And yeah, table cells are kind of a nuisance, but since tables are lazy this should only be a problem if the window is big, right?

[sghpjuikit]

Why do you not set the font in css?

Because font has its own settings in ui with font picker and all, allowing user to 'override' skin value. We could dynamically generate a css file with the font style and apply that, but that seems dirty.

May be relevant: /javafxports/openjdk-jfx/pull/424

since tables are lazy

Tables are only virtualized in vertical direction and many columns may bring about unexpected performance problems. This just in: (/javafxports/openjdk-jfx/issues/409). Anyway, there may be room for improvement for us in this regard. I will keep reducing the amount of styling necessary, but it is a long term goal.

[xeruf]

We could dynamically generate a css file with the font style and apply that, but that seems dirty.

That is exactly how I apply themes in https://github.com/Xerus2000/util/blob/master/javafx/src/xerus/ktutil/javafx/Themes.kt

[sghpjuikit]

That is exactly how I apply themes

Do you consider it better approach than Node.setStyle?

[xeruf]

I honestly have no idea, but it sounds like you do setStyle for every single node? That doesn't seem efficient to me.

This was the only way I could find to update the styling of the whole Scene, which then also automatically copies that styling to any child stages.

[sghpjuikit]

You use setStyle on the root of the scene (one node per window). The properties trickle down on their own since they are inherited. As for child windows, not sure. Hard to say which is better actually.

[sghpjuikit]

More optimizations done:

• default skin is now Main, which has no skin imports
• container controls are created lazily - they are almost never used and this saves quite a bit of memory

[sghpjuikit]

More optimizations:

• container controls are now disposed when hidden
• DirView image loader memory leak on widget.close() fixed
• ConfigField layout is lazy
• It is now possible to use a simple rating control skin that displays text instead of icons.

I will also experiment using generated css file approach to Node.setStyle for applying font. Would also enable optional overriding of the skin.

[sghpjuikit]

Closing as delivered.

I did some checking. I find current memory consumption ok. I'll work on more optimizations and memory leaks, but I consider this not an issue anymore.

Latest measurements as observed from OS task manager:

• my application instance with basic widgets (playback, playlist, library) and a lots of songs in the db consumes about 350MB, moving mostly within 320-400M, .
• using manual loaded widgets (not auto-loaded) for library puts me on 250MB
• loading empty library gets me to 170MB

The most memory hungry is:

• in memory db
  We could use normal db for queries, but I find it a waste of the effort right now. It would be nice to run fast fulltext search though. Perhaps in the future.
• ui tables
  Honestly, this annoys me to no end and I suspect we can do better.
• images
  This is not standard use case and only affects scenarios where user must expect higher memory consumption. Np here at this point.

Note:
Having 200k PseudoclassState instances is suspicious, but it does not cause much of a memory overhead.

[sghpjuikit]

Recently implemented feature that allows launching widgets in new process can help with memory footprint of long running instance as it isolates one-off widgets into different process.