allthingssecurity commented on code in PR #27793:
URL: https://github.com/apache/camel/pull/27793#discussion_r4265513310
##########
components/camel-wasm/pom.xml:
##########
@@ -54,6 +54,12 @@
<artifactId>wasi</artifactId>
<version>${endive-version}</version>
</dependency>
+ <!-- used with compile=true only: translates a module to JVM bytecode,
once per endpoint -->
+ <dependency>
+ <groupId>run.endive</groupId>
+ <artifactId>compiler</artifactId>
+ <version>${endive-version}</version>
+ </dependency>
Review Comment:
Thanks. This is question 1 in the description, and I would like a maintainer
to decide it before I change the pom. The trade-off as I see it: optional saves
about 0.55 MB of jars (compiler plus ASM, in the version Camel already manages)
for users who never set `compile=true`, but then `compile=true` does not work
out of the box: with `camel run` it needs `--dep
run.endive:compiler:<version>`, and a user finds that out from the
`IllegalStateException` at the first start. The code path and the test for the
missing jar are there, so switching to `<optional>true</optional>` is a
one-line change plus a sentence in the component page; I will make it if that
is the preferred default.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-wasm/src/main/java/org/apache/camel/wasm/WasiCommand.java:
##########
@@ -73,10 +91,77 @@ public final class WasiCommand {
* than the initial memory the module
declares
*/
public WasiCommand(WasmModule module, List<String> arguments, int
maxMemoryPages) {
+ this(module, arguments, maxMemoryPages, false);
+ }
+
+ /**
+ * @param module the parsed module
+ * @param arguments the program's arguments, the program
name first
+ * @param maxMemoryPages the maximum number of 64 KiB pages the
module's memory may grow to, or 0 to keep
+ * the maximum the module declares
+ * @param compile whether to translate the module to JVM
bytecode now, with the Endive runtime
+ * compiler, instead of interpreting it.
This defines classes at run time, which a
+ * GraalVM native image cannot do. A
function that is too large for a JVM method is
+ * left to the interpreter, which is
logged once, at WARN
+ * @throws IllegalArgumentException if the module is not a WASI Preview 1
command module, or maxMemoryPages is lower
+ * than the initial memory the module
declares
+ * @throws IllegalStateException if compile is set and the Endive
runtime compiler is not on the classpath, or
+ * cannot compile the module. The message
names the module by its program name (the
+ * first argument)
+ */
+ public WasiCommand(WasmModule module, List<String> arguments, int
maxMemoryPages, boolean compile) {
this.module = Objects.requireNonNull(module);
this.arguments = List.copyOf(arguments);
validate(module);
this.memoryLimits = memoryLimits(module, maxMemoryPages);
+
+ Function<Instance, Machine> factory = null;
+ boolean fallback = false;
+ if (compile) {
+ String name = this.arguments.isEmpty() ? "(no name)" :
this.arguments.get(0);
+ try {
+ try {
+ factory = RuntimeCompiler.compile(module, false);
+ } catch (WasmEngineException e) {
+ // Endive could not compile every function. When the
reason is a function that is too large for a
+ // JVM method, the module still works with that function
interpreted, so compile again with the
+ // interpreter as fallback; for any other reason this
second attempt fails as the first one did.
+ factory = RuntimeCompiler.compile(module, true);
+ fallback = true;
+ LOG.warn("The Wasm module {} is compiled in part only: it
has functions that are too large for a"
Review Comment:
Thanks. I chose WARN because it is logged once per endpoint, at the first
start, and it tells the operator that `compile=true` is only partly in effect
for this module, which they asked for and are not getting in full; nothing else
reports it. INFO would also be fine with me, since the module does run
correctly. It belongs to question 3 in the description (fallback with a log
line, silent fallback, or failure), so I will follow what the maintainers
prefer there and change the level in the same step.
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
