Skip to content

Commit bdd7fe9

Browse files
andrusandrus
committed
refactoring - shortcircuting CodeEvaluator/JShell integration
no need for an elaborate JJavaExecutionControlProvider, CodeEvaluator is an ExecutionControlProvider
1 parent 9d1cede commit bdd7fe9

5 files changed

Lines changed: 97 additions & 141 deletions

File tree

‎jjava-distro/src/main/java/org/dflib/jjava/distro/JJava.java‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
import java.nio.file.Path;
2121
import java.nio.file.Paths;
2222
import java.util.Properties;
23+
import java.util.concurrent.TimeUnit;
24+
import java.util.regex.Matcher;
25+
import java.util.regex.Pattern;
2326

2427
/**
2528
* The main class launching Jupyter Java kernel.
@@ -52,13 +55,15 @@ public static void main(String[] args) throws Exception {
5255
Properties pomProps = loadPomProps();
5356
TimeMagic timeMagic = new TimeMagic();
5457

58+
Timeout timeout = Timeout.parseOrDefault(System.getenv(Env.JJAVA_TIMEOUT));
59+
5560
JavaKernel kernel = JavaKernel.builder()
5661
.name("JJava")
5762
.version((String) pomProps.getOrDefault("version", ""))
5863

5964
.extensionsEnabled(Env.extensionsEnabled())
6065
.compilerOpts(Env.compilerOpts())
61-
.timeout(Env.timeout())
66+
.timeout(timeout.time, timeout.timeUnit)
6267

6368
.lineMagic("load", new LoadCodeMagic("", ".jsh", ".jshell", ".java", ".jjava"))
6469
.lineMagic("classpath", new ClasspathMagic())
@@ -113,4 +118,40 @@ private static Properties loadPomProps() {
113118
throw new RuntimeException("Error reading project properties");
114119
}
115120
}
121+
122+
private static class Timeout {
123+
private static final Pattern TIMEOUT_PATTERN = Pattern.compile("^(?<dur>-?\\d+)\\W*(?<unit>[A-Za-z]+)?$");
124+
125+
static Timeout parseOrDefault(String value) {
126+
127+
if (value == null) {
128+
return new Timeout(-1, TimeUnit.MILLISECONDS);
129+
}
130+
131+
Matcher m = TIMEOUT_PATTERN.matcher(value);
132+
if (!m.matches()) {
133+
throw new IllegalArgumentException("Invalid timeout string: " + value);
134+
}
135+
136+
long timeout = Long.parseLong(m.group("dur"));
137+
TimeUnit unit = TimeUnit.MILLISECONDS;
138+
if (m.group("unit") != null) {
139+
try {
140+
unit = TimeUnit.valueOf(m.group("unit").toUpperCase());
141+
} catch (IllegalArgumentException e) {
142+
throw new IllegalArgumentException("Invalid timeout unit: " + m.group("unit"));
143+
}
144+
}
145+
146+
return new Timeout(timeout, unit);
147+
}
148+
149+
final long time;
150+
final TimeUnit timeUnit;
151+
152+
public Timeout(long time, TimeUnit timeUnit) {
153+
this.time = time;
154+
this.timeUnit = timeUnit;
155+
}
156+
}
116157
}

‎jjava-kernel/src/main/java/org/dflib/jjava/kernel/JavaKernel.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
import jdk.jshell.SnippetEvent;
88
import jdk.jshell.SourceCodeAnalysis;
99
import jdk.jshell.UnresolvedReferenceException;
10-
import org.dflib.jjava.jupyter.kernel.BaseKernel;
1110
import org.dflib.jjava.jupyter.instrumentation.EvalTimer;
11+
import org.dflib.jjava.jupyter.kernel.BaseKernel;
1212
import org.dflib.jjava.jupyter.kernel.HelpLink;
1313
import org.dflib.jjava.jupyter.kernel.JupyterIO;
1414
import org.dflib.jjava.jupyter.kernel.LanguageInfo;
@@ -28,7 +28,6 @@
2828
import org.dflib.jjava.kernel.execution.EvaluationInterruptedException;
2929
import org.dflib.jjava.kernel.execution.EvaluationTimeoutException;
3030
import org.dflib.jjava.kernel.execution.IncompleteSourceException;
31-
import org.dflib.jjava.kernel.execution.JJavaExecutionControlProvider;
3231

3332
import java.nio.charset.Charset;
3433
import java.util.ArrayList;
@@ -369,10 +368,9 @@ public JavaKernel build() {
369368

370369
String name = buildName();
371370
Charset jupyterEncoding = buildJupyterIOEncoding();
372-
JJavaExecutionControlProvider execControlProvider = buildJShellExecControlProvider(name);
373-
CodeEvaluator evaluator = buildCodeEvaluator(name, execControlProvider);
371+
CodeEvaluator evaluator = buildCodeEvaluator(name);
374372

375-
JShell jShell = buildJShell(execControlProvider);
373+
JShell jShell = buildJShell(evaluator);
376374
LanguageInfo langInfo = buildLanguageInfo();
377375
MagicTranspiler magicTranspiler = buildMagicTranspiler();
378376

‎jjava-kernel/src/main/java/org/dflib/jjava/kernel/JavaKernelBuilder.java‎

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
import org.dflib.jjava.jupyter.kernel.magic.MagicTranspiler;
77
import org.dflib.jjava.jupyter.kernel.magic.MagicsResolver;
88
import org.dflib.jjava.kernel.execution.CodeEvaluator;
9-
import org.dflib.jjava.kernel.execution.JJavaExecutionControlProvider;
9+
import org.dflib.jjava.kernel.execution.JJavaExecutionControl;
10+
import org.dflib.jjava.kernel.execution.JJavaLoaderDelegate;
1011

1112
import java.util.ArrayList;
12-
import java.util.HashMap;
1313
import java.util.List;
1414
import java.util.Map;
15-
import java.util.UUID;
15+
import java.util.Objects;
16+
import java.util.concurrent.TimeUnit;
1617

1718
/**
1819
* A common builder superclass for JJavaKernel and subclasses.
@@ -21,58 +22,44 @@ public abstract class JavaKernelBuilder<
2122
B extends JavaKernelBuilder<B, K>,
2223
K extends JavaKernel> extends BaseKernelBuilder<B, K> {
2324

24-
protected final String jShellExecControlID;
25-
protected JJavaExecutionControlProvider jShellExecControlProvider;
26-
protected String timeout;
25+
protected long timeoutDuration;
26+
protected TimeUnit timeoutUnit;
2727
protected final List<String> compilerOpts;
2828

2929
protected JavaKernelBuilder() {
30-
this.jShellExecControlID = UUID.randomUUID().toString();
3130
this.compilerOpts = new ArrayList<>();
3231
}
3332

34-
public B jShellExecControlProvider(JJavaExecutionControlProvider jShellExecControlProvider) {
35-
this.jShellExecControlProvider = jShellExecControlProvider;
36-
return (B) this;
37-
}
38-
3933
public B compilerOpts(Iterable<String> opts) {
4034
opts.forEach(this.compilerOpts::add);
4135
return (B) this;
4236
}
4337

44-
/**
45-
* Sets the kernel communication timeout. The String should be a number with a {@link java.util.concurrent.TimeUnit}
46-
* name. E.g. "30 SECONDS"
47-
*/
48-
public B timeout(String timeout) {
49-
this.timeout = timeout;
38+
public B timeout(long timeoutDuration, TimeUnit timeoutUnit) {
39+
this.timeoutDuration = timeoutDuration;
40+
this.timeoutUnit = Objects.requireNonNull(timeoutUnit);
5041
return (B) this;
5142
}
5243

5344
@Override
5445
public abstract K build();
5546

56-
protected JShell buildJShell(JJavaExecutionControlProvider jShellExecControlProvider) {
57-
58-
Map<String, String> execControlParams = new HashMap<>();
59-
execControlParams.put(JJavaExecutionControlProvider.REGISTRATION_ID_KEY, jShellExecControlID);
60-
61-
if (timeout != null) {
62-
execControlParams.put(JJavaExecutionControlProvider.TIMEOUT_KEY, timeout);
63-
}
64-
47+
protected JShell buildJShell(CodeEvaluator evaluator) {
6548
return JShell.builder()
6649
.out(System.out)
6750
.err(System.err)
6851
.in(System.in)
69-
.executionEngine(jShellExecControlProvider, execControlParams)
52+
.executionEngine(evaluator.getExecControlProvider(), Map.of())
7053
.compilerOptions(compilerOpts.toArray(new String[0]))
7154
.build();
7255
}
7356

74-
protected CodeEvaluator buildCodeEvaluator(String name, JJavaExecutionControlProvider execControlProvider) {
75-
return new CodeEvaluator(name, execControlProvider, jShellExecControlID);
57+
protected CodeEvaluator buildCodeEvaluator(String name) {
58+
long timeoutDuration = this.timeoutUnit != null ? this.timeoutDuration : -1;
59+
TimeUnit timeoutUnit = this.timeoutUnit != null ? this.timeoutUnit : TimeUnit.MILLISECONDS;
60+
return new CodeEvaluator(
61+
name,
62+
new JJavaExecutionControl(new JJavaLoaderDelegate(), timeoutDuration, timeoutUnit));
7663
}
7764

7865
protected MagicsResolver buildMagicsResolver(MagicTranspiler transpiler) {
@@ -90,10 +77,4 @@ protected LanguageInfo buildLanguageInfo() {
9077
.codemirror("java")
9178
.build();
9279
}
93-
94-
protected JJavaExecutionControlProvider buildJShellExecControlProvider(String name) {
95-
return this.jShellExecControlProvider != null
96-
? jShellExecControlProvider
97-
: new JJavaExecutionControlProvider(name);
98-
}
9980
}

‎jjava-kernel/src/main/java/org/dflib/jjava/kernel/execution/CodeEvaluator.java‎

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@
66
import jdk.jshell.Snippet;
77
import jdk.jshell.SnippetEvent;
88
import jdk.jshell.SourceCodeAnalysis;
9-
import org.dflib.jjava.jupyter.kernel.BaseKernel;
9+
import jdk.jshell.spi.ExecutionControl;
10+
import jdk.jshell.spi.ExecutionControlProvider;
11+
import jdk.jshell.spi.ExecutionEnv;
1012
import org.dflib.jjava.jupyter.instrumentation.EvalTimer;
13+
import org.dflib.jjava.jupyter.kernel.BaseKernel;
1114

1215
import java.lang.reflect.InvocationTargetException;
1316
import java.lang.reflect.Method;
1417
import java.util.List;
18+
import java.util.Map;
1519
import java.util.regex.Matcher;
1620
import java.util.regex.Pattern;
1721

@@ -33,19 +37,16 @@ public class CodeEvaluator {
3337
}
3438
}
3539

36-
3740
private final String name;
38-
private final JJavaExecutionControlProvider execControlProvider;
39-
private final String execControlID;
40-
41-
public CodeEvaluator(
42-
String name,
43-
JJavaExecutionControlProvider execControlProvider,
44-
String execControlID) {
41+
private final JJavaExecutionControl execControl;
4542

43+
public CodeEvaluator(String name, JJavaExecutionControl execControl) {
4644
this.name = name;
47-
this.execControlProvider = execControlProvider;
48-
this.execControlID = execControlID;
45+
this.execControl = execControl;
46+
}
47+
48+
public ExecutionControlProvider getExecControlProvider() {
49+
return new SimpleExecControlProvider(name, execControl);
4950
}
5051

5152
public Object eval(JShell shell, String code, EvalTimer timer) {
@@ -70,7 +71,6 @@ public Object eval(JShell shell, String code, EvalTimer timer) {
7071

7172
protected Object evalSingle(JShell shell, String code, EvalTimer timer) {
7273

73-
JJavaExecutionControl execControl = execControlProvider.getRegisteredControlByID(execControlID);
7474
List<SnippetEvent> events = timer.runAndMeasureStep(() -> shell.eval(code));
7575

7676
Object result = null;
@@ -147,7 +147,6 @@ protected Object evalSingle(JShell shell, String code, EvalTimer timer) {
147147
* Try to clean up information linked to a code snippet and the snippet itself
148148
*/
149149
private void dropSnippet(JShell shell, Snippet snippet) {
150-
JJavaExecutionControl execControl = execControlProvider.getRegisteredControlByID(execControlID);
151150
shell.drop(snippet);
152151
// snippet.classFullName() returns name of a wrapper class created for a snippet
153152
String className = snippetClassName(snippet);
@@ -236,15 +235,31 @@ public String isComplete(SourceCodeAnalysis sourceAnalyzer, String code) {
236235
}
237236

238237
public void interrupt() {
239-
JJavaExecutionControl execControl = execControlProvider.getRegisteredControlByID(execControlID);
240-
241-
if (execControl != null) {
242-
execControl.interrupt();
243-
}
238+
execControl.interrupt();
244239
}
245240

246241
public ClassLoader getClassLoader() {
247-
JJavaExecutionControl execControl = execControlProvider.getRegisteredControlByID(execControlID);
248-
return execControl != null ? execControl.getClassLoader() : null;
242+
return execControl.getClassLoader();
243+
}
244+
245+
static final class SimpleExecControlProvider implements ExecutionControlProvider {
246+
247+
private final String name;
248+
private final ExecutionControl control;
249+
250+
public SimpleExecControlProvider(String name, ExecutionControl control) {
251+
this.name = name;
252+
this.control = control;
253+
}
254+
255+
@Override
256+
public String name() {
257+
return name;
258+
}
259+
260+
@Override
261+
public ExecutionControl generate(ExecutionEnv env, Map<String, String> parameters) {
262+
return control;
263+
}
249264
}
250265
}

‎jjava-kernel/src/main/java/org/dflib/jjava/kernel/execution/JJavaExecutionControlProvider.java‎

Lines changed: 0 additions & 79 deletions
This file was deleted.

0 commit comments

Comments
 (0)