[jsscripting] Improve performance & reduce memory usage

[florian-h05]

Description

Improve performance

Performance is improved by caching parts of the global code that is injected into each new script, see e4c9547.
Currently, only the @jsscripting-globals.js is cached, but if you disable the injection of globals into UI scripts inside the UI's settings, you can see the effect of this code caching: scripts are extremely fast evaluated, even on my Pi 3 the evaluation time is nearly zero.

For some background, see:

• https://www.graalvm.org/22.0/reference-manual/embed-languages/#code-caching-across-multiple-contexts
• https://www.graalvm.org/sdk/javadoc/

I would like to also cache the library code, but I am not sure how to do this while keeping the ability to reload the library during runtime of the addon. Since this will be a larger change, I won’t do this in this PR.

Reduce memory usage

This is mainly done by sharing one org.graalvm.polyglot.Engine across all OpenhabGraalJSScriptEngines, see a86e32a.

Also see oracle/graaljs#121 (comment), it is not required to have one engine per GraalJSScriptEngine.

This results in the following: With 5 GraalJS UI scripts, heap usage is now below 100 MB. Before this change, it was over 100 MB.

Concerns One Can Have

• Multi-threaded access problems: This is no problem with the current changes, see the conversation below with my test script.
• Memory leak: As far as I can see from my heap dumps, the current changes do not lead to any memory leak.

Current changes refers to what changed until commit e4c9547.

@digitaldan This might be interesting for you and I’d like to get your review here.

[J-N-K]

Will this affect the "closing" of the script engine that we recently introduced?

[florian-h05]

I don’t think so.
I have to explain this a little bit, because there are multiple engines:

• openHAB gets the OpenhabGraalJSScriptEngine which is a GraalJSScriptEngine with some extensions. Both implement ScriptEngine and the behaviour of this is not expected to change with this PR.
• To construct a GraalJSScriptEngine, you need a org.graalvm.polyglot.Engine and that can be shared acrross the GraalJSScriptEngines. This is what this PR does.

[J-N-K]

Hopefully this does not bring us into trouble with multi-threaded access, but I'm sure you checked that.

[florian-h05]

I don't expect it to do so, since the problem with multi-threaded access is coming from the polyglot Context and every engine still has its own Context, but of course I checked this:

I have two script files in the UI:

This one here checks whether multi-thread access is properly synchronized inside one OpenhabGraalJSScriptEngine (100 timers scheduled to the same time should lead to an exception if synchronization would not work):

var Thread = Java.type('java.lang.Thread');

// Create 100 timers for the same timeout to ensure that timers/scheduled are now threadsafe
for (var i = 0; i < 100; i++) {
  function gnr (index) {
    return () => {
      console.info('setTimeout ' + index + ' completed successfully');
      Thread.sleep(500);
    };
  };
  setTimeout(gnr(i), 100);
  }
// Ensure that the rule is still running when the timers expire
Thread.sleep(5000);

And the second one:
This ensures that there is a script locked for some amount of time (60 seconds).

var Thread = Java.type('java.lang.Thread');

console.warn('Starting Thread.sleep(60 sec)...')
Thread.sleep(60000);
console.warn('Finished with sleeping.')

The result:
The timers log their message one after the other without throwing an exception.
The same time, the other script is sleeping and there is no deadlock for the first script, no exception.

[jlaur]

This results in the following: With 5 GraalJS UI scripts, heap usage is now below 100 MB. Before this change, it was over 100 MB.

Just curious, can you share more here? I guess it was not 99,99 MB vs. 100,01 MB? :-)

[florian-h05]

Just curious, can you share more here? I guess it was not 99,99 MB vs. 100,01 MB? :-)

I currently don’t have my PC on so I can’t check, but IIRC it was something like 120MB vs 80MB.

[jlaur]

I currently don’t have my PC on so I can’t check, but IIRC it was something like 120MB vs 80MB.

Okay, that seems quite significant! The description of below and above 100 MB just really didn't tell much. 😄

[digitaldan]

LGTM!

[digitaldan]

I would like to also cache the library code, but I am not sure how to do this while keeping the ability to reload the library during runtime of the addon. Since this will be a larger change, I won’t do this in this PR.

i think this is a worth while exercise, I can take a look a little bit on this, but i'm sure you will beat me to a solution :-)

[jlaur]

Thank you!

[florian-h05]

i think this is a worth while exercise, I can take a look a little bit on this, but i'm sure you will beat me to a solution :-)

To improve performance with the library, we would also need to cache it the same way as the globals script.
But if we do that, I think we aren’t able to reload the library during runtime of the addon. It would load the library when the addon initialises and cache it for the next runs.

My idea is, to check on creation of the OpenhabGraalJSScriptEngine whether we have a node_modules/openhab folder in the hosts file system. If it exists evaluate Object.assign(this, require(„openhab“); (with bad performance). If it does not exist, evaluate the cached Source with great performance.

@digitaldan WDYT of that proposal?

[digitaldan]

We have a file watcher class to look for changes to the local node_modules dir, would it not be possible to thread safely update the shared Source object if the source changes on disk ?

[florian-h05]

I‘m not sure if that’s possible, I need to check that in the code .

[digitaldan]

So i started thinking about this in earnest, and realized that yeah, doing this might be very tough given how we have a different loading mechanism for loading packages out of node_modules (and its not a single webpacked file) . Your idea sounds reasonable, but i also wonder if we could make this part of the binding configuration instead? So you could opt out of the default shipped openhab-js? This to me would be a cleaner solution with less complexity.

[florian-h05]

Your idea sounds reasonable, but i also wonder if we could make this part of the binding configuration instead? So you could opt out of the default shipped openhab-js? This to me would be a cleaner solution with less complexity.

Yes, we can make it part of the binding configuration, and I agree that this would make it a cleaner solution with less complexity. I will see if I can implement this.

[florian-h05]

See #14135 for openhab-js injection caching.