feat: Improve set variable performance

[thedist]

Part of a set of changes (the other being to companion-module-base) to improve the performance of modules setting variables.

Currently, modules set variable values as an Object of key/value pairs, such as { someVariableId: 'variableValue' }, but the companion-module-base transforms this into an array such as [{ variableId: 'someVariableId, value: 'variableValue' }], and then after being sent from the child process to Companion it has to be transformed into { someVariableId: 'variableValue' } again. This iteration over the variables adds avoidable overhead.

A simple test I set up to generate an arrays of set amounts of variables took this long to transform to an object:
1,000 variables: 0.2ms
10,000 variables: 1.8344ms
100,00 variables: 23.3069ms
1,000,000 variables: 148.7271ms

Compared to if the object sent by the instance was already an object
1,000 variables: 0.0024ms
10,000 variables: 0.0006ms
100,000 variables: 0.0006ms
1,000,000 variables: 0.0014ms

This PR should be fully backwards compatible, so all existing modules would continue to send arrays of variables and need to be transformed, but any module that updates their companion module base (with a PR that will be coming shortly), it would be sending an object and bypass the transforming completely.

[Julusian]

I think the reason I did this is that sending { abc: undefined } over IPC will likely result in abc disappearing from the object entirely. Passing it as an array avoids this.

What would be the impact of leaving the object -> array conversion inside module-base, and passing it around as an array inside companion? We are simply iterating over the whole object, so it could easily remain as an array (will require some simple fixing up of other callsites of that method)

I suppose another hazard that would be nice to avoid is if a 'batch' of changes includes changing the same variable multiple times. It would be nice to optimise this out, but maybe we can reason that this should never happen, and that its a bug in module-base if it does

[thedist]

One of the reasons I didn't want it to remain an array is the performance improvements I'm testing in companion module base involves using a Map rather than an object (but trying to send a Map over IPC results in an empty object {}, and transforming it to a Map rather than an Object on Companions side didn't show much in the way of performance gains (at least not with the current setup).

Edit: I just checked and you are indeed correct, undefined couldn't be sent, but null can so perhaps that could be a solution for that particular issue.

The only time that undefined should be set is either if the module dev intentionally wants to remove a variable, or if validation is enabled and the value being set is for an variable id not defiend. These are quite niche situations so perhaps another option would be to remove these entirely from the object being sent from instance to companion, and providing a separate IPC message such as deleteVariableValues which is an array of variable id strings for Companion to delete.

[jswalden]

Letting the internal value encoding (or encoding constrained by IPC functionality) bleed into the user-facing UI seems like kind of a mistake. If you're going to require modules to affirmatively opt in to the newer, faster UI, maybe it makes sense to add a new, parallel UI -- and then you can implement the old slow UI in terms of the new one. (And by "new UI" maybe that also includes separating the variable-setting case from the variable-deleting case int two new functions.)

[thedist]

Letting the internal value encoding (or encoding constrained by IPC functionality) bleed into the user-facing UI seems like kind of a mistake. If you're going to require modules to affirmatively opt in to the newer, faster UI, maybe it makes sense to add a new, parallel UI -- and then you can implement the old slow UI in terms of the new one. (And by "new UI" maybe that also includes separating the variable-setting case from the variable-deleting case int two new functions.)

What user-facing UI are you talking about? This has nothing whatsoever to do with UI, or anything directly user-facing at all so I'm confused about your comment.

[thedist]

Converted to Draft while I'm working on some changes on the companion-module-base to assist in this.

[jswalden]

What user-facing UI are you talking about?

Sorry, by "user" I meant "module developer" as opposed to "Companion implementer". Didn't think of the obvious confusion that word would have here! In my head modules use Companion, so they/module developers are "users" of it, even if that's plainly not the best name to be thinking of here.

[thedist]

Well in terms of integration for Module Developers, I personally don't see what benefit it would have to split this into multiple ways for developers to have their module send variables to Companion.

Currently, module developers use instance.setVariableValues and pass it an object.
From there companion-module-base (still within the instance of that module) iterates over it and makes an array of objects which is then sent to Companion over IPC.
Companion takes that array and turns back into an object, and processes the variables internally within Companion.

The changes I'm working on are to 1. lower the overhead between companion-module-base and Companion, this would be entirely transparent to Module Developers, you'd still set your variables exactly the same way. and 2. updating setVariableValues within companion-module-base to support a Map, as on the module developers side this can have performance gains when generating the variables and values before they are even sent to setVariableValues, but currently a Map has to first be changed to an Object before passing to setVariableValues which has some overhead itself.

What will this mean for Module Developers?
if you want to send an object to setVariableValues like all modules currently do, that'll work without changes.
If you want to send an object to setVariableValues, but have a version of Companion and use companion-module-base with these changes, you'll have performance benefits without having to do anything.
If you want to send a Map to setVariableValues, that'll work too with a version of companion-module-base that supports it.
If you want to delete variables, again, use setVariableValues the same way it has been already and just use undefined as a value, how they are deleted would all be handled behind the scenes as Module Developers don't need to get into the specifics of it.

So my plans are to be full backwards compatible, no need for separate functions to send variables using the new way, no need for a separate function to delete variables.

[Julusian]

@thedist That all sounds fine with me.

I am still curious on this though:

What would be the impact of leaving the object -> array conversion inside module-base, and passing it around as an array inside companion? We are simply iterating over the whole object, so it could easily remain as an array (will require some simple fixing up of other callsites of that method)

How much does it cost to do the object/map to array conversion there, especially compared to alternatives. It sounds like its going to have to do some kind of conversion (to handle both object and map, and to preserve setting to undefined), so it feels like keeping it transported as an array could be the best option.
The key thing we can do is minimise the cost inside companion, as the array to object change is pretty pointless.

If you are going to change the ipc format, dont feel like you need to make it work within the existing ipc message. Adding a new one with a different parameter layout is perfectly fine. I did this already in I think 1.2 for how we send updated configs to modules