companion icon indicating copy to clipboard operation
companion copied to clipboard

[BUG] WebUI out of mem with a large number of instance variable definitions

Open thedist opened this issue 3 years ago • 5 comments

Is this a bug in companion itself or a module?

  • [X] I believe this to be a bug in companion

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

While working on the Google Sheets module I have set up instance variables for each cell, such as SpreadsheetName_sheet!A1, and have found that while the number of instance variables scales quite well, their definitions do not.

I do not currently know the exact point at which this becomes an issue, but with my current testing in the Google Sheets module it has about 100,000 cells, but with this many definitions it causes the browser tab showing the WebUI to run out of memory and crash either when attempting to view the variables tab for the module, or when starting to type an instance variable for a button and the autocomplete starts loading the definitions list.

As this is just a limitation of Instance Variable Definitions, I think one solution to this issue may be to create a hard limit within Companion on the number of definitions a module can register by simply slicing the array of definitions sent by the module to some limit (and maybe logging a warning). This way regardless of what a module attempts to do Companion wont let it negatively impact the WebUI.

Steps To Reproduce

  1. Create a module
  2. Use a loop to create a large number of Instance Variable Definitions
  3. Try browse the instance variables, or start typing one on a button, and monitor the tabs mem usage

Expected Behavior

Expected behaviour would be for instance variable definitions to be limited so that at no point can a module break the WebUI (or any core part of Companion)

Environment (please complete the following information)

- OS: Win 10
- Browser: Chrome
- Companion Version: 2.2.2 beta

Additional context

No response

thedist avatar Jun 25 '22 15:06 thedist

I expect that the point it becomes an issue will vary depending on the machine viewing the webui. And potentially also the network between the two, as a large number of variables will be a large blob of data to transfer when loading the page.

It would be interesting to know where all the memory is going. Is it the size of the json, feeding that into tribute, the dropdowns which use variables or something else?

But yes a good starting point for this would be to put some limit on the number of variables that a module can expose

Julusian avatar Jun 25 '22 15:06 Julusian

Alternatively, we could look at whether we could do more with dynamic loading of the variables instead. We could likely get away with not loading the full list of variable-definitions to the client, and instead doing queries to the backend as the user types. It might make the ui feel a bit slower when doing this, but equally I have noticed that the dropdowns are noticably laggy at times

Julusian avatar Jul 03 '22 17:07 Julusian

Could a hybrid approach of sorts work? Rather than doing queries each letter to the backend, or having the entire instance variable list on the client, we could instead have it so as you start typing ${ it'll just use the list of instances to show all of the instance labels, eg ${internal:, ${http:, ${vmix: etc... and when you type one of the labels it'll then make a request to the backend to for the instance variables of that instance.

The benefit of this is that it'd limit it to only ever trying to display, and filter as you type, just the instance variables for 1 instance. Should 1 instance have an insane amount of variables it wont break trying to use variables from any other instance, and should still have the responsiveness of filtering as the user types client-side.

The downside to this would be no instance variable suggestions until a user types the whole instance label.

Additionally, one way to perhaps further help would be to add a category property to instance variables. Much like when browsing presets they are not only divided into instances, but each module can categorise presets. Same thing could be done with instance variables, and for backwards compatibility any instance variable without a category could be always shown rather than nested in a category. So for example if I'm going to the Variables tab, rather than just going to a 'Google Sheets' tab and seeing thousands of variables, it could show me a few uncategorised variables (such as status type things), and a category for each sheet within a spreadsheet, which would limit how many variables are being displayed/filtered at any one time in the client.

thedist avatar Jul 04 '22 12:07 thedist

With a couple of simple ui changes I am able to run with 750,000 definitions. Things are definitely a bit slow, and sometimes loading the ui does timeout (even when connecting over localhost).
Companion itself is perfectly fine with this number, and is running at around 700mb of memory (I didnt give them values, just the definitions). It looks to be pushing ~80mb of json to the ui for this.

80mb is a lot of data to be pushing to the ui, but it only really becomes problematic when we try and draw them all at once in the DOM. My simple fix has been to cap the number that we will use at once: image.

I have found that even without this ridiculous number of variables, that the text input with variables suggestions feels rather sluggish. It might be time to start properly investigating replacing tributejs with something more react friendly (https://www.npmjs.com/package/react-autocomplete-input looks reasonable). Trying to use a variable in the internal feedbacks which present a dropdown is causing chrome the chrome tab to be unresponsive.

I've been thinking more on whether we can avoid loading the full list of variable definitions into the ui, but I have thought of some more complications:

  • I really like that it is possible to find a variable by searching for a keyword, you don't have to fill in the instance label first
  • Anywhere that a variable is used may need to load in the variable definitions for any instance (internal actions/feedbacks/triggers)
  • The definitions will need to be cached for a little while, to avoid churn of unloading them briefly. (This should be fine to do)

With all that, I'm not sure what the best solution here is.

Julusian avatar Nov 15 '22 22:11 Julusian

I wonder how responsive it would be to offload autocomplete entirely to the Backend.

For example, as you start typing if the string includes the start of an Instance variable $( with no closing paren to end it, the element could send the current text string through the websocket to Companion under an 'autocomplete' event. Companion could then check through the instance variables as it would always have all of them loaded, and then respond with 5 to 10 results (and an indicator that there's x number of results not shown). As the users enters more characters it'll progressively narrow down the results. This will mean that the frontend wont need to have all instance variables loaded.

The only time the frontend would need to work with a large set of instance variables would be when going to the variables page, but this again could be broken down into smaller data sets by only requesting from Companion just the variables for the category, eg 'Custom Variables', or for one of the modules. For modules with very large number of variables (Google Sheets) we could add a 'category' value to instance variables, so that when I go to the variable page for Google Sheets, rather than having hundreds of thousands of variables, I would instead get another set of buttons to go to one of the specific categories, with all variables lacking a category going to a 'default' page to allow for backwards compatibility for all existing modules.

thedist avatar Nov 20 '22 14:11 thedist