Croway commented on code in PR #26308:
URL: https://github.com/apache/camel/pull/26308#discussion_r3988949729
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsLanguage.java:
##########
@@ -130,17 +140,62 @@ private Object eval(String script, Map<String, Object>
bindings) {
EngineState state = currentEngine();
state.stderr.reset();
try {
- return state.engine.invokeGuestFunction(
+ Object result = state.engine.invokeGuestFunction(
QuickjsHelper.MODULE_NAME,
QuickjsHelper.FUNCTION_NAME,
List.of(bindings, script),
QuickjsHelper.EVAL_WRAPPER);
+ if (state.exhausted(engineMaxMemory, engineMaxEvaluations)) {
+ discard(state);
+ }
+ return result;
} finally {
// Drop this evaluation's WASI stderr so a reused Engine cannot
accumulate it.
state.stderr.reset();
}
Review Comment:
Confirmed and fixed in e3de79728884: the check now runs in the finally
block, so a throwing evaluation counts and is measured like a successful one.
Added throwingScriptsCountTowardsRecycling: 10 failing evaluations with
engineMaxEvaluations=10 leave trackedEngineCount() at zero and the next
evaluation gets a fresh engine.
_Claude Code on behalf of Croway_
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsLanguage.java:
##########
@@ -130,17 +140,62 @@ private Object eval(String script, Map<String, Object>
bindings) {
EngineState state = currentEngine();
state.stderr.reset();
try {
- return state.engine.invokeGuestFunction(
+ Object result = state.engine.invokeGuestFunction(
QuickjsHelper.MODULE_NAME,
QuickjsHelper.FUNCTION_NAME,
List.of(bindings, script),
QuickjsHelper.EVAL_WRAPPER);
+ if (state.exhausted(engineMaxMemory, engineMaxEvaluations)) {
+ discard(state);
+ }
+ return result;
} finally {
// Drop this evaluation's WASI stderr so a reused Engine cannot
accumulate it.
state.stderr.reset();
}
}
+ /**
+ * Closes the calling thread's engine; the next evaluation on this thread
creates a fresh one.
+ */
+ private void discard(EngineState state) {
+ if (engine.get() == state) {
+ engine.remove();
+ }
+ engines.remove(state.engine);
Review Comment:
Applied in e3de79728884: discard() only closes the engine when it was the
one to remove it from the queue.
_Claude Code on behalf of Croway_
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsLanguage.java:
##########
@@ -47,6 +48,15 @@
@Language("quickjs")
public class QuickjsLanguage extends TypedLanguageSupport implements
ScriptingLanguage, Service {
+ /**
+ * Every evaluation executes a module in the QuickJS runtime, and QuickJS
keeps evaluated modules until its context
+ * is freed, so an engine grows with every evaluation (about 12 KB each).
An engine is therefore recycled once its
+ * WebAssembly memory exceeds {@link #getEngineMaxMemory()} or it has run
{@link #getEngineMaxEvaluations()}
+ * evaluations: it is closed and the thread creates a fresh one on its
next evaluation.
+ */
+ private long engineMaxMemory = 64L * 1024 * 1024;
+ private int engineMaxEvaluations = 50_000;
Review Comment:
Applied in e3de79728884: both fields are volatile.
_Claude Code on behalf of Croway_
--
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]
