[jsscripting] Cache openhab-js injection to improve performance #14135

florian-h05 · 2023-01-01T22:00:15Z

For previous discussion see #14113.

Description

This PR extends the code caching to the version of the openHAB JavaScript library (openhab-js) that is included in the JS Scripting addon.
A new configuration option is introduced to allow the user to disable usage of the internal openhab-js library and instead load it from the file system.

~~Note: I am currently not super happy with the description and the label of that new configuration option, I would like to get some feedback on them.~~

Code caching of the openhab-js injection brings great performance improvements as the evaluation of the pretty large openhab-js codebase takes some time.

Note: it is not possible to cache the Object.assign(this, require("openhab"); therefore I had to take the way of creating an injection script at openhab-js.

I've performed a few test runs on my dev system (that is quite powerful, AMD Ryzen 7) and here are the results:

openhab-js injection without caching: about 1 second
openhab-js injection with code caching: between 100 and 200 milliseconds

On my Raspberry Pi 3 (Debian 10 Buster), caching makes a even larger difference:

openhab-js injection without caching: about 13 seconds
openhab-js injection with code caching: about 2 seconds

Note that evaluation of the cached version takes a bit longer than usual after the first start of the addon -- the code needs to be evaluated once before it is cached.

To Do

Regenerate default translations
Maybe improve the label and description of the new configuration option

Ping @digitaldan @jlaur

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

On my dev system (which I guess is much more powerful than most openHAB servers), cached openhab-js injection takes 100-200 ms. openhab-js injection from file system takes about 1000 ms. This depends on changes to the openhab-js library that have not been released yet. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

digitaldan

Great work! I have a few comments regarding configuration language as you requested, nothing major. Changes in code looks good to me, was planning on getting my 4.0 environment up today and see if i can test this.

bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml

Also-by: Dan Cunningham >dan@digitaldan.com> Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 · 2023-01-02T17:10:05Z

Great work! I have a few comments regarding configuration language as you requested, nothing major.

Thanks :-)

Changes in code looks good to me, was planning on getting my 4.0 environment up today and see if i can test this.

This PR depends on openhab/openhab-js#218.
FYI: For testing, I generated the webpacked openhab-js with the openhab-js PR and copied it to the target/js/dist/ folder of the addon. I also locally removed the module bundling from the pom to lower build time. Just use Maven without the clean command then.

jlaur · 2023-01-02T17:16:23Z

@florian-h05 - great work again. I'm wondering if it would be possible to also cache the library from node_modules and watch the directory in order to invalidate the cached version upon changes? Since latest version is even provided through openHABian, this is quite convenient and I could imagine many "ordinary" users preferring to use this. I already switched and will probably never look back. :-)

florian-h05 · 2023-01-02T17:27:31Z

great work again

Thanks :-)

I'm wondering if it would be possible to also cache the library from node_modules and watch the directory in order to invalidate the cached version upon changes?

I was trying this, but unfortunantely code caching for loaded code (via require) doesn't work (doesn't improve performance). This is because every engine has it's own CommonJS module loader. I was thinking if it would be possible to share the module loader because it is also caching in some ways, but I did not find a docs to do this and looking at the GraalJS code, it seems that the loader is bound to the script.

Since latest version is even provided through openHABian, this is quite convenient and I could imagine many "ordinary" users preferring to use this.

This is mainly thought for openhab-js updates in case you need a yet not addon included fix or enhancement.
Note that this change makes the addon use the included library unless you explicitely disable this in the addon settings.

I have the feeling that openhab-js is now in a well-tested and stable state and I hope that in the future there are not as many changes as in the past, so I hope when 4.0 is released in summer, you can live with the included version and won't miss something from the latest openhab-js.

…k changes Documentation updates will follow in another PR to keep this one clean. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

digitaldan

LGTM

jlaur

Thanks. Only a minor comment about consistent naming.

bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml

...ng/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

...les/org.openhab.automation.jsscripting/src/main/resources/OH-INF/i18n/jsscripting.properties

Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Jacob Laursen <jacob-github@vindvejr.dk>

jlaur

Thank you!

digitaldan · 2023-01-02T21:50:50Z

@florian-h05 does require("openhab") no longer work when dealing with other libraries in the node_modules? I have a helper script which i use by first requiring our lib like:

const ns = {};

// assign everything in @oh, so items, actions, triggers, etc....
Object.assign(ns, require("openhab"));
.....
module.exports = ns;

i then add some proxy magic for my own helper functions and export the whole thing. This is throwing errors that it can't find openhab. Did changing the name (@jsscripting-globals) prevent using our lib by the openhab name ?

florian-h05 · 2023-01-02T21:57:33Z

Oh yes, I see the problem.
The problem is that there is no openhab file in the addon anymore. If you install openhab-js to the disk, it will work again.
I will provide a fix for this.

…#14135 Fixes the regression from openhab#14135 (comment), I noticed that the cached openhab-js injection does not work. While working on this, I also noticed that the cache openhab-js does not work because of wrong webpack commandline args in the pom (wrong entrypoint). Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…#14135 Fixes the regression from openhab#14135 (comment). While working on this, I also noticed that the cache openhab-js does not work because of wrong webpack commandline args in the pom (wrong entrypoint). Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…mentException` (#14142) * [jsscripting] Fix bundling of global script & regression from #14135 Fixes the regression from #14135 (comment). While working on this, I also noticed that the cache openhab-js does not work because of wrong webpack commandline args in the pom (wrong entrypoint). * [jsscripting] Enable stack logging for IllegalArgumentExceptions * [jsscripting] Upgrade openhab-js to 3.2.4 * [jsscripting] Update README for recent PR Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

openhab-bot · 2023-01-07T18:45:05Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/oh3-to-use-or-not-to-use-js-ecmascript-2021/141911/11

…hab#14135) * [jsscripting] Extend comments for wraprequire * [jsscripting] Enable openhab-js caching to improve performance On my dev system (which I guess is much more powerful than most openHAB servers), cached openhab-js injection takes 100-200 ms. openhab-js injection from file system takes about 1000 ms. * [jsscripting] Update configuration language * [jsscripting] Upgrade openhab-js version to 3.2.1 for required webpack changes Documentation updates will follow in another PR to keep this one clean. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…galArgumentException` (openhab#14142) * [jsscripting] Fix bundling of global script & regression from openhab#14135 Fixes the regression from openhab#14135 (comment). While working on this, I also noticed that the cache openhab-js does not work because of wrong webpack commandline args in the pom (wrong entrypoint). * [jsscripting] Enable stack logging for IllegalArgumentExceptions * [jsscripting] Upgrade openhab-js to 3.2.4 * [jsscripting] Update README for recent PR Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…hab#14135) * [jsscripting] Extend comments for wraprequire * [jsscripting] Enable openhab-js caching to improve performance On my dev system (which I guess is much more powerful than most openHAB servers), cached openhab-js injection takes 100-200 ms. openhab-js injection from file system takes about 1000 ms. * [jsscripting] Update configuration language * [jsscripting] Upgrade openhab-js version to 3.2.1 for required webpack changes Documentation updates will follow in another PR to keep this one clean. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…galArgumentException` (openhab#14142) * [jsscripting] Fix bundling of global script & regression from openhab#14135 Fixes the regression from openhab#14135 (comment). While working on this, I also noticed that the cache openhab-js does not work because of wrong webpack commandline args in the pom (wrong entrypoint). * [jsscripting] Enable stack logging for IllegalArgumentExceptions * [jsscripting] Upgrade openhab-js to 3.2.4 * [jsscripting] Update README for recent PR Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…hab#14135) * [jsscripting] Extend comments for wraprequire * [jsscripting] Enable openhab-js caching to improve performance On my dev system (which I guess is much more powerful than most openHAB servers), cached openhab-js injection takes 100-200 ms. openhab-js injection from file system takes about 1000 ms. * [jsscripting] Update configuration language * [jsscripting] Upgrade openhab-js version to 3.2.1 for required webpack changes Documentation updates will follow in another PR to keep this one clean. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…galArgumentException` (openhab#14142) * [jsscripting] Fix bundling of global script & regression from openhab#14135 Fixes the regression from openhab#14135 (comment). While working on this, I also noticed that the cache openhab-js does not work because of wrong webpack commandline args in the pom (wrong entrypoint). * [jsscripting] Enable stack logging for IllegalArgumentExceptions * [jsscripting] Upgrade openhab-js to 3.2.4 * [jsscripting] Update README for recent PR Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

openhab-bot · 2023-11-17T08:07:25Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/oh3-to-use-or-not-to-use-js-ecmascript-2021/141911/24

florian-h05 added 2 commits January 1, 2023 22:48

[jsscripting] Extend comments for wraprequire

130944e

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 added the enhancement An enhancement or new feature for an existing add-on label Jan 1, 2023

florian-h05 requested a review from digitaldan January 1, 2023 22:00

This was referenced Jan 1, 2023

[jsscripting] Improve performance & reduce memory usage #14113

Merged

Update bundling mechanism for globals injection caching openhab/openhab-js#218

Merged

florian-h05 added the awaiting other PR Depends on another PR label Jan 1, 2023

digitaldan requested changes Jan 2, 2023

View reviewed changes

florian-h05 added 2 commits January 2, 2023 18:03

[jsscripting] Update configuration language

448b796

Also-by: Dan Cunningham >dan@digitaldan.com> Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

[jsscripting] Regenerate default translations

b3be7b8

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 requested a review from digitaldan January 2, 2023 17:30

[jsscripting] Upgrade openhab-js version to 3.2.1 for required webpac…

c8af56a

…k changes Documentation updates will follow in another PR to keep this one clean. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 removed the awaiting other PR Depends on another PR label Jan 2, 2023

digitaldan approved these changes Jan 2, 2023

View reviewed changes

jlaur requested changes Jan 2, 2023

View reviewed changes

florian-h05 added 2 commits January 2, 2023 20:15

[jsscripting] Change configuration name for consistency

4aee102

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

[jsscripting] Update JavaDoc

b742202

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

jlaur reviewed Jan 2, 2023

View reviewed changes

...les/org.openhab.automation.jsscripting/src/main/resources/OH-INF/i18n/jsscripting.properties Outdated Show resolved Hide resolved

[jsscripting] Remove obsolete default translations

eb34b69

Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Jacob Laursen <jacob-github@vindvejr.dk>

jlaur approved these changes Jan 2, 2023

View reviewed changes

jlaur merged commit 3c669ad into openhab:main Jan 2, 2023

jlaur added this to the 4.0 milestone Jan 2, 2023

florian-h05 deleted the jsscripting-cache-openhab-js branch January 2, 2023 19:51

florian-h05 mentioned this pull request Jan 3, 2023

[jsscripting] Fix regressions from #14135 & Log stack on IllegalArgumentException #14142

Merged

florian-h05 mentioned this pull request Jan 5, 2023

[jsscripting] Update docs for code caching #14166

Merged

florian-h05 mentioned this pull request Nov 17, 2023

[jsscripting] GraalVM's optimised compiler or find another solution #15600

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jsscripting] Cache openhab-js injection to improve performance #14135

[jsscripting] Cache openhab-js injection to improve performance #14135

florian-h05 commented Jan 1, 2023 •

edited

Loading

digitaldan left a comment •

edited

Loading

florian-h05 commented Jan 2, 2023

jlaur commented Jan 2, 2023

florian-h05 commented Jan 2, 2023 •

edited

Loading

digitaldan left a comment

jlaur left a comment

jlaur left a comment

digitaldan commented Jan 2, 2023

florian-h05 commented Jan 2, 2023

openhab-bot commented Jan 7, 2023

openhab-bot commented Nov 17, 2023

[jsscripting] Cache openhab-js injection to improve performance #14135

[jsscripting] Cache openhab-js injection to improve performance #14135

Conversation

florian-h05 commented Jan 1, 2023 • edited Loading

Description

To Do

digitaldan left a comment • edited Loading

Choose a reason for hiding this comment

florian-h05 commented Jan 2, 2023

jlaur commented Jan 2, 2023

florian-h05 commented Jan 2, 2023 • edited Loading

digitaldan left a comment

Choose a reason for hiding this comment

jlaur left a comment

Choose a reason for hiding this comment

jlaur left a comment

Choose a reason for hiding this comment

digitaldan commented Jan 2, 2023

florian-h05 commented Jan 2, 2023

openhab-bot commented Jan 7, 2023

openhab-bot commented Nov 17, 2023

florian-h05 commented Jan 1, 2023 •

edited

Loading

digitaldan left a comment •

edited

Loading

florian-h05 commented Jan 2, 2023 •

edited

Loading