Compare memory consumption between FW8 and latest V10

[pleku]

Use latest Flow 'n components snapshots.

[denis-anisimov]

Several observations about memory consumption in Patient portal:

• The most memory is occupied by the backend data. This data is Patient objects and JournalEntry objects. I see 100 Patient objects in memory per session with common size ~180Kb and around of 1000 JournalEntry objects with common size ~120Kb.
• Those objects are referenced from KeyMapper of DataCommunicator of Grid inside PatientsView.
• I don't know whether this is the issue of lazy loading in Grid, DataProvider or Grid itself. But this is definitely issue related to Grid ONLY.
• Another issue which relates to the big amount of memory consumed by the Grid instance is that PatientsView stays referenced in the application even if the user navigates to another view (which may be very small an doesn't consume a lot of memory by itself).

I don't know how FW8 version works in these aspects:

• I doubt that reported memory consumed by application in FW8 is real: the backend data requires significant memory even being loaded lazily (it won't be 100 Person objects but still quite many). May be patients view has not been measured at all in FW8 case or see the next item
• It might be that FW8 version doesn't keep reference to the patients list view after navigation to another view (analytics view e.g.). In this case all (backend) data referenced from Grid is GCed along with Grid and that's the reason why FW8 application doesn't consume so much memory.

As a result there are two aspects/questions:

• Why Grid does reference so many backend objects ? (is 100 Patients expected number of loaded objects). This issue is pure Grid issue and memory consumption should be investigated for Grid explicitly and separately.
• Do we really keep all navigation targets in memory even if they are not shown at the moment (most likely yes, we keep them to be able to reuse).
• In case we do this : should we give to the developer control over this "navigation target" cache ? Patient portal use case shows that Grid may consumes a lot of memory and it should not be cached in case this view is a regular view without requirement to keep it in memory expecting its reuse. So our logic in the navigator should allow to GC views somehow (introduce scopes/add heuristics so that view is cached only for some time if it's not reused/ introduce a way to mark a navigation target as not cacheable ?)

[Legioth]

Keeping 100 items in memory is expected because the server needs to keep a reference to anything that he client has in its cache so that the server can find the right instance if the user selects something through the UI. Having 100 items available for immediate scrolling doesn't seem like too much.

The navigation target being retained after navigating away is not expected and could be further investigated.

[denis-anisimov]

Actually the reason of retained patient list is (at least) the application design:
MainView is annotated with @UIScope. PatientsView which contains the Grid has @ParentLayout(MainView.class) which means that it's added to the MainView instance.

It might be that this behavior is not really expected since PatientsView is not annotated by the @UIScope itself and the fact that it's added to the MainView instance and will be a part of its component hierarchy is not obvious.

[denis-anisimov]

On the other hand AnalyticsView also has MainView as a parent layout and should replace the PatientsView instance after the navigation.
But I don't see the removal of the previous content in the RouterLayout logic:

default void showRouterLayoutContent(HasElement content) {
        if (content != null) {
            getElement().appendChild(Objects.requireNonNull(content.getElement()));
        }
    }

So on the client side it places the component in the slot (replaces the client side component) but on the server side it doesn't remove the previous component and it stays referenced in the children list.

[Legioth]

Detaching the old child happens outside RouterLayout. UIInternals.showRouteTarget calls removeFromParent(oldChild) right before calling showRouterLayoutContent.

[denis-anisimov]

Aha, OK.

[denis-anisimov]

Right, there is no Grid anymore referenced after the navigation to another view.
The Patient instances are referenced from the KeyMapper in the DataCommunicator instance which is referenced from some DataCommunicator lambda which is in the list of task for executions inside StateTree.StateNodeOnBeforeClientResponse. This node is in the list of executionsToProcessBeforeResponse.

[Legioth]

executionsToProcessBeforeResponse should be using WeakReference for individual nodes to avoid exactly this kind of situation? If this is the only reference, then it might be that the WeakReference hasn't yet had time to be reclaimed? Or then there's also some other reference in some other place?

[Legioth]

No wait. The problem is probably that the Runnable (i.e. the lambda) has also captured a reference to the same node, which thus prevents the WeakReference from being released, which in turn also prevents the StateNodeOnBeforeClientResponse entry from being released. And we cannot also wrap the Runnable in a WeakReference since there are genuinely no other references to it.

[Legioth]

I'm afraid this means that we'll need a different way of keeping beforeClientResponse tasks around for nodes that are detached so that they can be triggered later on if the node gets attached again. Storing the tasks separately in each node would be problematic since that would mean that the ordering would be completely lost.

We can obviously not preserve ordering between tasks for nodes that have never been attached, so for those it's completely fine that the ordering is based on when they are attached.

[pleku]

I'm afraid this means that we'll need a different way of keeping beforeClientResponse tasks around for nodes that are detached so that they can be triggered later on if the node gets attached again.

While this should be fixed anyway, I wonder if the following thing could be handled in a better manner in DataCommunicator since it is probably the cause of these references:

    private void requestFlush() {
        if (flushRequest == null) {
            flushRequest = () -> {
                flush();
                flushRequest = null;
            };
            stateNode.runWhenAttached(ui -> ui.getInternals().getStateTree()
                    .beforeClientResponse(stateNode, flushRequest));
        }
    }

    private void requestFlushUpdatedData() {
        if (flushUpdatedDataRequest == null) {
            flushUpdatedDataRequest = () -> {
                flushUpdatedData();
                flushUpdatedDataRequest = null;
            };
            stateNode.runWhenAttached(ui -> ui.getInternals().getStateTree()
                    .beforeClientResponse(stateNode, flushUpdatedDataRequest));
        }
    }