RFR: 8290040: Provide simplified deterministic way to manage listeners [v6]

Nir Lisker nlisker at openjdk.org
Wed Oct 19 20:18:03 UTC 2022


On Tue, 18 Oct 2022 18:44:58 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>>> I'd respectfully disagree: in a large application, there might be hundreds (a thousand?) Nodes, and only a handful (10-20?) Nodes with this property (2%), so it's a waste of RAM. One may think it's only 8Kb on a 64 bit machine, but it does add up quickly. Even the access pattern is basically 'get it once to establish the fluent pipeline', with no more calls to the Node accessor (unlike MiscProperties constituents, some of which get accessed rather frequently).
>> 
>> I understand your point, but I don't think that this is sufficient reason to introduce a new way of storing property instances in an ad-hoc manner. If memory consumption is a concern, I would rather analyze the issue in a separate enhancement proposal, think it through in terms of API, and then transition all relevant property instances to the new storage mechanism. Part of that proposal should also be an evaluation of the trade-offs between memory consumption and performance in terms of cache locality and lookup speed.
>
> @mstr2 : 
> `getProperties()` is a perfectly fine way of storing a property instance, especially considering the fact that this property will be called exactly once - during the fluent pipeline setup.  Of course, it does not apply to other properties that are being called all the time.
> 
> See ButtonBarSkin:136
> 
> A separate ticket for optimizing property storage might be a good idea, I agree.

> > `getProperties()` isn't the place for this.
> 
> can you explain why?

This map is used for storing values for the node. As the API note says, layout managers use this to store constraints that be updated by the user. I think that this is already not the best way to do it. The user can also store arbitrary data like markers or other associations. Storing a `Property` is like storing functionality, not data.

In addition, the map is publicly accessible, so the user can clear it. In this case you risk GC of the instance. This is equivalent to making a property field both `public` and not `final`, allowing the user to overwrite the reference - a rather disastrous scenario.

-------------

PR: https://git.openjdk.org/jfx/pull/830


More information about the openjfx-dev mailing list