Enhancement #6233

DependencyChangesRequest holds a strong reference on to its parameter org.simantics.db.ChangeSet which can pile up in QueryProcessor and consume a lot of memory

Added by Tuukka Lehtonen over 2 years ago. Updated about 17 hours ago.

Status:ClosedStart date:2018-02-13
Priority:4Due date:2018-02-27
Assignee:Antti Villberg% Done:

100%

Category:DB clientSpent time:-
Target version:2018-04
Release notes:Minimized ChangeSet structure resource usage after the change sets are no longer used. This prevents @DependencyChangesRequest@ from keeping strong references to large amounts of already useless data just because the requests can be cached in @QueryProcessor@ for quite a long period of time before garbage collection kicks the requests out.
Tags: perf, leak, 1.33.0
Story points-
Velocity based estimate-
ReleaseSimantics 1.33.0Release relationshipAuto

Description

Currently in an Apros stress test we have an undo test case where a script tries to perform the following operations on every diagram element in every model configuration diagram:
  1. transform the diagram element a bit
  2. undo

When this sequence is executed at full speed without any sleep, the DB client starts piling up DepdenciesRelation.DependencyChangeRequest instances in its query cache (readMap). The DependencyChangesRequest instances are shallowly not very large but the retained heap size is pretty much the amount of memory retained by class ClientChangesImpl, which is at least 128KB. In the mentioned test, the JVM ran into OOM, when it had around 22500 instances of DependencyChangesRequests cached resulting in a total retained heap of almost 3GB just because of these ClientChangesImpl instances.

Proposal:
  1. Make it possible to retrieve change set ID through ChangeSet interface
  2. Change the identity of DependencyChangesRequests from ChangeSet to ChangeSet ID
  3. Nullify ChangeSet parameter from DependencyChangesRequest after its execution

This will work because each DependencyChangesRequest for a certain change set will be cached for the time when each of GenericChangeListener is invoked that instantiates DependencyChangesRequest.

Of course this does not help with the fact that currently scripts can execute changes at such a rapid rate that the DB client caches will consume all JVM memory and the DB client query cache collector just does not have enough time to free up the necessary cache entries. However, this would be substantial reduction of used memory in cases where lots of requests are performed via script, which can happen also when executing model creation scripts etc.

Associated revisions

Revision 09366c70
Added by Tuukka Lehtonen about 1 month ago

Dispose ClientChangesImpl ChangeSets to minimize memory footprint

This change stems from the fact that
org.simantics.db.common.changeset.GenericChangeListener gives ChangeSets
as an argument to DB requests which can leave strong references to the
ChangeSet instances in the DB QueryProcessor until the DB client
garbage collects these requests. Usually DB client request parameters
should be immutable, but in this case the request is never listened to
for changes so it is OK dispose of the ChangeSet parameter after all
DB ChangeListeners are notified.

This change adds a Disposable implementation to ClientChangesImpl which
minimizes the memory footprint of the class down to 192 bytes compared
to ~2^17 bytes. The instances can take up a very considerable amount of
memory if not disposed. It has not been uncommon to see multiple
gigabytes of memory being spent by these byte[] buffers that are not
really used anymore but still referenced.

Once more note that this is not a case of memory leakage but simply bad
use of memory through over-sized buffers that were GC'ed eventually.

refs #6233

Change-Id: I2e96754f106602a4986c37187a9af3bbd64356dc

Revision 1dd02a0d
Added by Tuukka Lehtonen about 1 month ago

Defer change set disposal in State.commitWriteTransaction

The change set was still used after disposal in notifying db change
listeners.

refs #6233

Change-Id: I3e9f49ea084703afdc2129162402ab592f2f3f72

History

#1 Updated by Tuukka Lehtonen almost 2 years ago

  • Release changed from 31 to 39

#2 Updated by Tuukka Lehtonen over 1 year ago

  • Release deleted (39)

#3 Updated by Tuukka Lehtonen over 1 year ago

  • Tracker changed from Bug to Enhancement

#4 Updated by Tuukka Lehtonen over 1 year ago

  • Status changed from New to On hold

#5 Updated by Tuukka Lehtonen over 1 year ago

  • Tags changed from leak to leak, perf

#6 Updated by Tuukka Lehtonen about 1 year ago

I'd recommend fixing this one way or another ASAP (preferable for 1.28.0 still). I believe that this would avoid some current cases of OOM and SCL-scripting high memory consumption problems.

#7 Updated by Tuukka Lehtonen 2 months ago

  • Release set to 58

#8 Updated by Tuukka Lehtonen 2 months ago

  • Due date set to 2018-02-27
  • Status changed from On hold to Feedback
  • Target version set to 2018-03
  • Start date set to 2018-02-13

Made a draft change for this: https://www.simantics.org:8088/r/#/c/1474/

Jani, could you test this out?

I decided to go with a solution that disposes the ClientChangesImpl instances after the system is done with them. I'm not sure whether I've covered all angles but hopefully at least most of them.

#9 Updated by Tuukka Lehtonen 2 months ago

  • Tags changed from leak, perf to leak, perf, 1.33.0

#10 Updated by Tuukka Lehtonen about 1 month ago

  • % Done changed from 0 to 100
  • Release notes set to Minimized ChangeSet structure resource usage after the change sets are no longer used. This prevents @DependencyChangesRequest@ from keeping strong references to large amounts of already useless data just because the requests can be cached in @QueryProcessor@ for quite a long period of time before garbage collection kicks the requests out.

#11 Updated by Tuukka Lehtonen about 1 month ago

  • Target version changed from 2018-03 to 2018-04

#12 Updated by Tuukka Lehtonen about 17 hours ago

  • Status changed from Feedback to Closed

Also available in: Atom PDF