A longer answer will take more time than I have at the moment, but here
is a quick reply.
1. We don't have a lot of scene graph performance tests, so before
making any changes here we would need to do some targeted testing.
2. FX scene graph node --> NGNode peer is intended to be be one-way
3. There are certainly use cases for reusing nodes (attach / detach).
VirtualFlow will do this for ListView, TableView, etc., with many rows.
I would expect some apps to make use of it as well, since it's cheaper
than recreating the FX nodes. There are also cases where nodes are moved
from one parent to another and might transiently be "removed" from the
graph.
4. Setting the node's peer to null when detached will cause it to be
recreated when / if the node is added back into the scene graph, which
you touch on later in your email. We wouldn't want the case of a
scrolling ListView / TableVIew to have to continually recreate peers as
the cells are cycled through.
Anything done here would need to be done very carefully to ensure both
correctness and performance. Your idea of splitting the peer data from
the graph connectivity is interesting, but it also sounds pretty intrusive.
Before going down that route I'd want to understand the problem better
and see if there was a simpler solution to clear the parent / child
relationship in the render graph.
-- Kevin
On 2/2/2023 9:17 AM, John Hendrikx wrote:
I have a few questions that maybe some Prism/SceneGraph experts could
help me with.
1) Are there any performance tests related to syncing the NGNode
peers? Specifically, I'm interested if there are tests that compare
the performance of "fresh" FX graphs that have never been displayed
before, versus ones that have their peers already created.
2) Does prism code ever refer back to FX Nodes? I've noticed that
NGGroup imports javafx.scene.Node for one of its method signatures,
but that seems to be a mistake; it can easily be changed to not
require javafx.scene.Node. Aside from several enums, constants and
data classes (like Color) being re-used from the javafx side, it seems
the NG Prism nodes are well encapsulated and that references are one
way only (FX Nodes refer to NG Nodes via `peer`, but never the other
way around).
3) How common is it to re-use FX Nodes that are no longer part of an
active scene? I've found myself that it is unwise to detach/recreate
children in high performance controls that reuse cells -- it's often
better to just hide cells that are not currently needed instead of
removing them from the children list.
4) Are there any (major) performance implications to setting the NG
peer to `null` when FX nodes are not part of an active sync cycle (ie,
they have no Scene, or the Scene is not currently visible)?
My observations on the sync cycle (syncPeer/doUpdatePeer) is that it
is highly optimized, and tries to avoid new allocations as much as
possible. However, this seems to come at a price: it leaks memory
when not part of a sync cycle. Given a FX Node graph, that has been
displayed at least once, a mirrored graph is created consisting of
NGNodes. When such a FX Node graph is no longer displayed, any changes
to it are no longer synced to the mirror.
This means that when I detach a small portion of the FX Node graph
(let's say a single Node), and keep a reference to only that Node,
that the FX graph can be GC'd. The corresponding NG Node graph however
(which still hasn't been updated) can't be GC'd. The single detached
FX Node has a peer, and this peer has a parent (and children), keeping
not only the detached Node's peer in memory, but also all the other
peers in the mirrored graph.
Notifying the NG mirrored graph of changes is not easy, as it must be
done as part of the sync (locking is involved). Even a detached FX
Node can't assume its peer can be modified without locking as it may
still be used in an active rendering pass.
This leads me to believe that to move to a situation where peers are
not being leaked would involve clearing peers as soon as FX nodes
becomes detached from the sync cycle.
This has no doubt has performance implications, as peers would need to
be recreated if nodes are re-used (see Q3). However, not all data
that is part of the peers is a problem, and code that simply clears
its peer when detached could be optimized again to be more peformant
(optimizing from a correctly working situation, instead of fixing
problems working from an optimized situation that has memory leaks).
One way to optimize this may be to split the data that is tracked by
peers in parts that are just direct copies of FX Node data, and parts
that refer to parent/children. The data that is just copies could be
kept ("peerData") while the peer itself is nulled. When the peer
requires recreation, it is constructed as "new NGNode(peerData)".
Because the parent/child linkages are not part of `peerData`, it is no
longer possible for large NG node trees to be kept in memory.
Recreation of NGNode would still require work, but these NGNodes are
much smaller.
--John
