[rules] Add support for pre-compilation of conditions and actions #4289

florian-h05 · 2024-06-30T22:05:01Z

This PR adds the ability for the compilation of rule script conditions and script actions on openHAB start-up or rule creation.

This is achieved by:

Creating the ScriptEngine on start-up or rule-creation and check if the ScriptEngine implements Compilable.
Call Compilable.compile(script) and store the returned Compiledscript on openHAB start-up or rule creation for ScriptConditionHandler and ScriptActionHandler.
Execute the CompiledScript instead of evaluating the script string if available.

Script automation add-ons need to make sure that their ScriptEngine implements the Compilable interface.

The motivation behind this PR is to allow JS scripts (as for example created by Blockly) to be compiled before they are executed, so there is no delay for the first run, which is a huge UX improvement.
From a performance POV, the compilation on start-up behaves like all rules with pre-compilable script conditions/actions would have start level triggers and would run on start-up, but without actually executing the script.

JS Scripting implementation: openhab/openhab-addons#16970

florian-h05 · 2024-07-01T08:34:52Z

@digitaldan FYI, while you are working on the great notification enhancements, I am not resting ;-)
Would be great if you could have a look and let me know what you think!

digitaldan · 2024-07-01T15:23:33Z

Awesome ! I actually saw #16970 first and asked a dumb question there, which could have been answered if i bothered reading that this was a dependency to it ;-)

I'll take a look today !

digitaldan

LGTM!

J-N-K · 2024-07-05T17:46:13Z

...dule.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineFactory.java

+     *
+     * @return true if pre-compilation is supported, else false
+     */
+    default boolean supportsPreCompilation() {


Why do you need this method? Wouldn't it be sufficient to check if the script engine implements Compilable?

This would work as well, but then I would need to always initialize the script engine when the script action/condition is created, even if it does not implement Compilable. At the moment, the script engine is initialised on first use.

Is that really an issue? If the engine implements Compilable there is no extra-cost. If it doesn't it just shifts the time when the engine is created from first usage to startup (which probably also improves performance when the script is actually called).

That's a good argument.
I thought there might be a reason it was implemented this way, but creating the engine at start up actually seems like a good idea. This also avoids that the memory usage seems to constantly grow over time.

I'll implement that.

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

…lation of scripts Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…ctivating a rule Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

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

florian-h05 · 2024-07-06T11:55:36Z

@J-N-K I implemented the requested changes.
Is there a way to delay the RuleEngine start-up until automation add-ons are ready?

Otherwise the log will get spammed with:

13:49:05.117 [ERROR] [ript.internal.ScriptEngineManagerImpl] - ScriptEngine for language 'application/javascript' could not be found for identifier: bd2f2674-6fa4-4f2c-bf8c-f79409d8b692
13:49:23.622 [INFO ] [re.automation.internal.RuleEngineImpl] - Rule engine started.

It of course would be possible to add a check to the AbstractScriptModuleHandler class to early return from compileScript if no engine factory is available.

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

florian-h05 · 2024-07-06T12:22:27Z

I went ahead and do now skip the compilation if no script engine is avalailable for the language.

florian-h05 · 2024-07-09T17:17:16Z

Thanks for merging!

florian-h05 · 2024-07-09T17:28:13Z

@jimtng FYI, not sure if JRuby Scripting need adjustments.
I have seen that it implements Compilable, but I am not sure if it needs a change because of its call interception to save the context into a global variable.

jlaur · 2024-07-14T13:39:34Z

@florian-h05 - I'm starting to see this in logs, is this expected?

2024-07-14 15:33:06.940 [ERROR] [.handler.AbstractScriptModuleHandler] - Script engine of rule with UID 'script' does not implement Compilable but claims to support pre-compilation

I think it happens when loading DSL rules, I tried saving an empty rule:

2024-07-14 15:38:25.269 [INFO ] [el.core.internal.ModelRepositoryImpl] - Loading model 'eds.rules'
2024-07-14 15:38:25.327 [ERROR] [.handler.AbstractScriptModuleHandler] - Script engine of rule with UID 'script' does not implement Compilable but claims to support pre-compilation

florian-h05 · 2024-07-14T19:42:21Z

No, this seems like a left-over from a previous version of this PR.
I will provide a PR to clean that up.

florian-h05 · 2024-07-14T19:45:17Z

See #4319.

Fixes an issue, where an error that compilation failed for disabled rules. Reported on the community: https://community.openhab.org/t/oh-4-2-snapshot-disabled-rules-failed-to-compile-error-in-opehab-log/157402. Follow-up for #4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Fixes openhab#17165. Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…UI scripts (#17171) * [jsscripting] Fix console logger name is missing for UI scripts * [jsscripting] Fix timer identifier "missing" for setTimeout/setInterval for UI scripts Fixes #17165. Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…UI scripts (openhab#17171) * [jsscripting] Fix console logger name is missing for UI scripts * [jsscripting] Fix timer identifier "missing" for setTimeout/setInterval for UI scripts Fixes openhab#17165. Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

openhab-bot · 2024-09-05T20:55:48Z

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

https://community.openhab.org/t/blockly-historic-state-average-ending-with-type-error/158191/11

…UI scripts (openhab#17171) * [jsscripting] Fix console logger name is missing for UI scripts * [jsscripting] Fix timer identifier "missing" for setTimeout/setInterval for UI scripts Fixes openhab#17165. Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>

…UI scripts (openhab#17171) * [jsscripting] Fix console logger name is missing for UI scripts * [jsscripting] Fix timer identifier "missing" for setTimeout/setInterval for UI scripts Fixes openhab#17165. Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 marked this pull request as ready for review June 30, 2024 22:08

florian-h05 requested a review from a team as a code owner June 30, 2024 22:08

florian-h05 mentioned this pull request Jun 30, 2024

[jsscripting] Implement javax.script.Compilable openhab/openhab-addons#16970

Merged

wborn added rebuild Triggers the Jenkins PR build and removed rebuild Triggers the Jenkins PR build labels Jul 1, 2024

holgerfriedrich added rebuild Triggers the Jenkins PR build and removed rebuild Triggers the Jenkins PR build labels Jul 1, 2024

digitaldan approved these changes Jul 1, 2024

View reviewed changes

kaikreuzer added the enhancement An enhancement or new feature of the Core label Jul 3, 2024

J-N-K reviewed Jul 5, 2024

View reviewed changes

florian-h05 added 4 commits July 6, 2024 12:43

Add ScriptEngineManager::isPreCompilationSupported

c0afb3d

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

ScriptConditionHandler/ScriptActionHandler: Add support for pre-compi…

d1b1d0b

…lation of scripts Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

RuleEngineImpl: Implement compilation of rules on start-up and when a…

4688e50

…ctivating a rule Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Address review

41bcd8d

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

Skip script compilation if no ScriptEngine found for language

c33942f

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

J-N-K merged commit 918b4fa into openhab:main Jul 9, 2024
5 checks passed

florian-h05 deleted the script-precompilation branch July 9, 2024 17:13

J-N-K added this to the 4.3 milestone Jul 9, 2024

florian-h05 mentioned this pull request Jul 14, 2024

[rules] Remove leftover from rfc in #4289 #4319

Merged

J-N-K pushed a commit that referenced this pull request Jul 14, 2024

[rules] Remove leftover from #4289 (#4319)

f631f1b

jimtng mentioned this pull request Jul 24, 2024

[jrubyscripting] Inject ctx in compiled scripts openhab/openhab-addons#17140

Merged

florian-h05 mentioned this pull request Jul 24, 2024

[rules] Don't attempt to pre-compile disabled rules #4329

Merged

florian-h05 mentioned this pull request Jul 27, 2024

[jsscripting] Logger name missing for UI-based rules openhab/openhab-addons#17165

Closed

florian-h05 added a commit to florian-h05/openhab-addons that referenced this pull request Jul 28, 2024

[jsscripting] Fix console logger name is missing for UI scripts

27dcb18

Fixes openhab#17165. Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 mentioned this pull request Jul 28, 2024

[jsscripting] Fix console logger name & timer identifier missing for UI scripts openhab/openhab-addons#17171

Merged

florian-h05 added a commit to florian-h05/openhab-js that referenced this pull request Jul 28, 2024

[actions] Fix missing timer identifier for UI scripts

3497353

Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 mentioned this pull request Jul 28, 2024

[actions] Fix missing timer identifier for UI scripts openhab/openhab-js#367

Merged

florian-h05 added a commit to openhab/openhab-js that referenced this pull request Jul 28, 2024

[actions] Fix missing timer identifier for UI scripts (#367)

795862a

Caused by openhab/openhab-core#4289. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 mentioned this pull request Oct 5, 2024

[jsscripting] Fix multi-threading issues in UI-based scripts openhab/openhab-addons#17510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rules] Add support for pre-compilation of conditions and actions #4289

[rules] Add support for pre-compilation of conditions and actions #4289

florian-h05 commented Jun 30, 2024 •

edited

Loading

florian-h05 commented Jul 1, 2024

digitaldan commented Jul 1, 2024

digitaldan left a comment

J-N-K Jul 5, 2024

florian-h05 Jul 5, 2024

J-N-K Jul 5, 2024

florian-h05 Jul 5, 2024

florian-h05 commented Jul 6, 2024

florian-h05 commented Jul 6, 2024

florian-h05 commented Jul 9, 2024

florian-h05 commented Jul 9, 2024

jlaur commented Jul 14, 2024

florian-h05 commented Jul 14, 2024

florian-h05 commented Jul 14, 2024

openhab-bot commented Sep 5, 2024

[rules] Add support for pre-compilation of conditions and actions #4289

[rules] Add support for pre-compilation of conditions and actions #4289

Conversation

florian-h05 commented Jun 30, 2024 • edited Loading

florian-h05 commented Jul 1, 2024

digitaldan commented Jul 1, 2024

digitaldan left a comment

Choose a reason for hiding this comment

J-N-K Jul 5, 2024

Choose a reason for hiding this comment

florian-h05 Jul 5, 2024

Choose a reason for hiding this comment

J-N-K Jul 5, 2024

Choose a reason for hiding this comment

florian-h05 Jul 5, 2024

Choose a reason for hiding this comment

florian-h05 commented Jul 6, 2024

florian-h05 commented Jul 6, 2024

florian-h05 commented Jul 9, 2024

florian-h05 commented Jul 9, 2024

jlaur commented Jul 14, 2024

florian-h05 commented Jul 14, 2024

florian-h05 commented Jul 14, 2024

openhab-bot commented Sep 5, 2024

florian-h05 commented Jun 30, 2024 •

edited

Loading