Optim: oNode attributes lazy loading

[Tilix4]

Problem

Creating oNode wrappers is expensive because the constructor eagerly loads ALL node attributes and creates getter/setters for each one, even when only basic properties like name or enabled are needed.

This causes significant performance issues when creating many node wrappers (e.g., getNodesByType calls).

Solution

Defer attribute loading until first access of the attributes property:

• Constructor no longer calls refreshAttributes()
• Attributes are loaded lazily on first .attributes access
• Getter/setters are created lazily on first access

Impact

Node wrapper creation is now nearly instant. Attributes are only loaded when actually needed.

Test

Regular use of API, more significant on big scenes.

[mchaptel]

I like what you're going for here but there are a couple of things that confuse me.
Does this change not break the creation/access of getter setters? Also have you tried enabling global caching? It's a bit experimental (because performance has never been an issue on my end big enough to actually go much deeper into this) but I did add a system to cache oNode objects upon creation.

You can enable it by setting this value to true :

OpenHarmony/openHarmony/base.js

Line 419 in 559989a

$.useCache = false;

Would you be ok with adding a few tests for this so we can make sure we're not breaking anything?

[Tilix4]

Hi, there are no collision between global caching and my lazy loading proposal. When $.useCache = true: The same instance is reused, and its lazy-loaded state is preserved:

1. First access to .attributes → loads them
2. Subsequent uses of same cached instance → already loaded

The lazy loading state is per-instance, so whether the instance is fresh or cached, the behavior is correct.
I'm building a 2D pipeline over Ayon, and its ayon-harmony addon has OpenHarmony as base API. I had to deal with perfomance issues in this context, because for publishing it crawls through all nodes in order to catch the ones marked to be published.

Also, I haven't tested your global caching feature very deeply but by reading the code I've found two issues which look critical to me:

1. Incomplete Cache Key in oNode

function oNode ( path, oSceneObject ){  
var instance = this.$.getInstanceFromCache.call(this, path);  
if (instance) return instance;

Only path is passed to getInstanceFromCache, but oSceneObject is also a constructor parameter. If two different scene objects requested the same path, they'd incorrectly share the same cached instance.

2. Incomplete Cache Key in oColumn

function oColumn (uniqueName, oAttributeObject){  
var instance = this.$.getInstanceFromCache.call(this, uniqueName);  
if (instance) return instance;

Same issue — only uniqueName is passed, ignoring oAttributeObject.

Are they left undefined on purpose?

[mchaptel]

I get that they are caching different things, I'm just worried about the getter setters directly on the node... If you can write a few tests for it that would be very reassuring and save me the time to go deep into it and we could merge it quicker.

I'm talking about things like doing

$.scn.selectedNodes[0].position.x = 5
instead of
$.scn.selectedNodes[0].attributes.position.x.setValue(5)

The scene object should really always be the same; to be honest I would happily remove that argument in the oNode creator now that the lib is more mature (anyway users should never create oNodes themselves). Most people access it through $.scn which is an instance created when the lib is loaded. We could easily make it a singleton instead I guess...

Harmony only allows one path and one name for node and columns respectively. So I think in practice this is fine. Actually the passing of AttributeObject is a bit of an architectural issue because depending on the way the column is accessed, it can't always know what it is associated to.

Ex: no way to know the attribute when getting $.scn.columns but you can if you do $.scn.selectedNodes[0].attributes.position.x.column

There are places in the code where some workaround was added, in frames manipulation/value setting and things like that.

Do you get perf problems that have an impact on your pipeline? It might be useful to measure the impact of that since on my side openHarmony has never been a relevant bottleneck, and the added complexity is accepted as a tradeoff for the code readibility and nomenclature consistency Nevermind just saw your mention of Ayon

I had to deal with perfomance issues in this context, because for publishing it crawls through all nodes in order to catch the ones marked to be published.

To be clear openHarmony stated goal is to enable scripting from the point of view of people with less background knowledge to access a more intuitive and complete set of features than the native api. I wonder if the ayon code should instead selectively switch to the native api for performance sensitive tasks? Something like selecting a set of nodes based on names I guess could be done this way? Not saying that improving performance in openHarmony wouldn't be nice but it won't always be possible. Note that all of this is irrelevant if we can just validate by test that this doesn't break anything

[Tilix4]

Oh sure, I misunderstood your first test request, I thought "this" was your global caching system. I wrote tests that you can run with the regular $.tests.run();.

Your concern was right, my previous implementation was requiring the user to access the attribute using attributes.param first before being able to access using param directly. In order to fix that I've added a startup code which initializes getters and setters placeholders without running the whole attributes wrapping. It's a bit slower but far much faster than without lazy loading.

I understand your point about Ayon and the philosophy of OH. I started by modifying Ayon's code by calling the native API in the first place TBH. But then I realized I could propose something to OpenHarmony which would benefits the whole API, and here we are.

[mchaptel]

But then I realized I could propose something to OpenHarmony which would benefits the whole API, and here we are.

I appreciate that!

[mchaptel]

Wow this is sure inventive but I'm really not sure we can create all the nodes that exist in the software just to init one node!!

If this works for you on your fork that's great but I think maybe we can wait for the nodeType prototype dynamic creation implementation instead of adding something like that even as a temporary workaround!

[Tilix4]

For sure, I really prefer the design you proposed.

[mchaptel]

is this a recent addition? I don't have this function available in harmony 24

[Tilix4]

Oh, this is actually very strange, I can see this function in harmony local documentation (through Help menu), while it's not in the online doc... And it works in the API. 😱

[mchaptel]

By the way the tests look great, but yeah that workaround seems really dicey to me to merge right now since we have a cleaner solution in mind already. I almost wish I could merge the tests only!

[Tilix4]

I almost wish I could merge the tests only!

Granted, 2 remaining.
#82