Skip to content

Commit 7737446

Browse files
andrusandrus
committed
refactoring - ExecutionControl brain surgery
.. passing LoaderDelegate to super, and using super methods where possible
1 parent 0f03199 commit 7737446

3 files changed

Lines changed: 85 additions & 113 deletions

File tree

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

Lines changed: 71 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import java.util.UUID;
1212
import java.util.concurrent.CancellationException;
1313
import java.util.concurrent.ConcurrentHashMap;
14-
import java.util.concurrent.ConcurrentMap;
1514
import java.util.concurrent.ExecutionException;
1615
import java.util.concurrent.ExecutorService;
1716
import java.util.concurrent.Executors;
@@ -39,158 +38,129 @@ public class JJavaExecutionControl extends DirectExecutionControl {
3938
*/
4039
public static final String EXECUTION_INTERRUPTED_NAME = "Execution Interrupted";
4140

42-
private static final Object NULL = new Object();
43-
4441
private static final AtomicInteger EXECUTOR_THREAD_ID = new AtomicInteger(0);
4542

4643
private final ExecutorService executor;
47-
48-
private final long timeoutTime;
44+
private final long timeoutDuration;
4945
private final TimeUnit timeoutUnit;
46+
private final Map<String, Future<Object>> running;
47+
private final Map<String, Object> results;
48+
private final JJavaLoaderDelegate loaderDelegate;
5049

51-
private final ConcurrentMap<String, Future<Object>> running = new ConcurrentHashMap<>();
52-
private final Map<String, Object> results = new ConcurrentHashMap<>();
50+
public JJavaExecutionControl(JJavaLoaderDelegate loaderDelegate, long timeoutDuration, TimeUnit timeoutUnit) {
51+
super(loaderDelegate);
5352

54-
private final JJavaLoaderDelegate loaderDelegate;
53+
this.loaderDelegate = loaderDelegate;
54+
this.running = new ConcurrentHashMap<>();
55+
this.results = new ConcurrentHashMap<>();
5556

56-
public JJavaExecutionControl() {
57-
this(-1, TimeUnit.MILLISECONDS);
57+
this.timeoutDuration = timeoutDuration;
58+
this.timeoutUnit = timeoutDuration > 0 ? Objects.requireNonNull(timeoutUnit) : TimeUnit.MILLISECONDS;
59+
this.executor = Executors.newCachedThreadPool(r -> new Thread(r, "JJava-executor-" + EXECUTOR_THREAD_ID.getAndIncrement()));
5860
}
5961

60-
public JJavaExecutionControl(long timeoutTime, TimeUnit timeoutUnit) {
61-
super(null);
62-
this.loaderDelegate = new JJavaLoaderDelegate();
63-
this.timeoutTime = timeoutTime;
64-
this.timeoutUnit = timeoutTime > 0 ? Objects.requireNonNull(timeoutUnit) : TimeUnit.MILLISECONDS;
65-
this.executor = Executors.newCachedThreadPool(r -> new Thread(r, "JJava-executor-" + EXECUTOR_THREAD_ID.getAndIncrement()));
62+
/**
63+
* Returns JShell ClassLoader
64+
*/
65+
public ClassLoader getClassLoader() {
66+
return loaderDelegate.getClassLoader();
6667
}
6768

6869
public long getTimeoutDuration() {
69-
return timeoutTime;
70+
return timeoutDuration;
7071
}
7172

7273
public TimeUnit getTimeoutUnit() {
7374
return timeoutUnit;
7475
}
7576

77+
/**
78+
* This method was hijacked and actually only returns a key that can be later retrieved via
79+
* {@link #takeResult(String)}. This should be called for every invocation as the objects are saved and not taking
80+
* them will leak the memory.
81+
*
82+
* @returns the key to use for {@link #takeResult(String) looking up the result}.
83+
*/
84+
@Override
85+
protected String invoke(Method doitMethod) throws Exception {
86+
String id = UUID.randomUUID().toString();
87+
Object value = execute(id, doitMethod);
88+
results.put(id, value);
89+
return id;
90+
}
91+
92+
@Override
93+
public void stop() {
94+
executor.shutdownNow();
95+
}
96+
7697
public Object takeResult(String key) {
7798
Object result = this.results.remove(key);
78-
if (result == null)
99+
if (result == null) {
79100
throw new IllegalStateException("No result with key: " + key);
80-
return result == NULL ? null : result;
101+
}
102+
103+
return result;
104+
}
105+
106+
public void unloadClass(String className) {
107+
loaderDelegate.unloadClass(className);
108+
}
109+
110+
public void interrupt() {
111+
running.forEach((id, f) -> f.cancel(true));
112+
}
113+
114+
@Override
115+
public String toString() {
116+
return "JJavaExecutionControl{" +
117+
"timeoutTime=" + timeoutDuration +
118+
", timeoutUnit=" + timeoutUnit +
119+
'}';
81120
}
82121

83122
private Object execute(String key, Method doitMethod) throws Exception {
84-
Future<Object> runningTask = this.executor.submit(() -> doitMethod.invoke(null));
85123

86-
this.running.put(key, runningTask);
124+
Future<Object> runningTask = executor.submit(() -> doitMethod.invoke(null));
125+
running.put(key, runningTask);
87126

88127
try {
89-
if (this.timeoutTime > 0)
90-
return runningTask.get(this.timeoutTime, this.timeoutUnit);
91-
return runningTask.get();
128+
return timeoutDuration > 0 ? runningTask.get(this.timeoutDuration, this.timeoutUnit) : runningTask.get();
92129
} catch (CancellationException e) {
93130
// If canceled this means that stop() or interrupt() was invoked.
94-
if (this.executor.isShutdown())
131+
if (executor.isShutdown()) {
95132
// If the executor is shutdown, the situation is the former in which
96133
// case the protocol is to throw an ExecutionControl.StoppedException.
97134
throw new StoppedException();
98-
else
135+
} else {
99136
// The execution was purposely interrupted.
100137
throw new UserException(
101138
"Execution interrupted.",
102139
EXECUTION_INTERRUPTED_NAME,
103-
e.getStackTrace()
104-
);
140+
e.getStackTrace());
141+
}
105142
} catch (ExecutionException e) {
106143
// The execution threw an exception. The actual exception is the cause of the ExecutionException.
107144
Throwable cause = e.getCause();
108145
if (cause instanceof InvocationTargetException) {
109146
// Unbox further
110147
cause = cause.getCause();
111148
}
112-
if (cause == null)
149+
if (cause == null) {
113150
throw new UserException("null", "Unknown Invocation Exception", e.getStackTrace());
114-
else if (cause instanceof SPIResolutionException)
151+
} else if (cause instanceof SPIResolutionException) {
115152
throw new ResolutionException(((SPIResolutionException) cause).id(), cause.getStackTrace());
116-
else
153+
} else {
117154
throw new UserException(String.valueOf(cause.getMessage()), cause.getClass().getName(), cause.getStackTrace());
155+
}
118156
} catch (TimeoutException e) {
119157
throw new UserException(
120-
String.format("Execution timed out after configured timeout of %d %s.", this.timeoutTime, this.timeoutUnit.toString().toLowerCase()),
158+
String.format("Execution timed out after configured timeout of %d %s.", this.timeoutDuration, this.timeoutUnit.toString().toLowerCase()),
121159
EXECUTION_TIMEOUT_NAME,
122160
e.getStackTrace()
123161
);
124162
} finally {
125-
this.running.remove(key, runningTask);
163+
running.remove(key, runningTask);
126164
}
127165
}
128-
129-
/**
130-
* This method was hijacked and actually only returns a key that can be
131-
* later retrieved via {@link #takeResult(String)}. This should be called
132-
* for every invocation as the objects are saved and not taking them will
133-
* leak the memory.
134-
* <p></p>
135-
* {@inheritDoc}
136-
*
137-
* @returns the key to use for {@link #takeResult(String) looking up the result}.
138-
*/
139-
@Override
140-
protected String invoke(Method doitMethod) throws Exception {
141-
String id = UUID.randomUUID().toString();
142-
Object value = this.execute(id, doitMethod);
143-
this.results.put(id, value);
144-
return id;
145-
}
146-
147-
public void interrupt() {
148-
running.forEach((id, f) -> f.cancel(true));
149-
}
150-
151-
@Override
152-
public void stop() {
153-
executor.shutdownNow();
154-
}
155-
156-
@Override
157-
public void load(ClassBytecodes[] cbcs) throws ClassInstallException {
158-
loaderDelegate.load(cbcs);
159-
}
160-
161-
@Override
162-
public void addToClasspath(String cp) throws InternalException {
163-
loaderDelegate.addToClasspath(cp);
164-
}
165-
166-
/**
167-
* Finds the class with the specified binary name.
168-
*
169-
* @param name the binary name of the class
170-
* @return the Class Object
171-
* @throws ClassNotFoundException if the class could not be found
172-
*/
173-
@Override
174-
protected Class<?> findClass(String name) throws ClassNotFoundException {
175-
return loaderDelegate.findClass(name);
176-
}
177-
178-
void unloadClass(String className) {
179-
this.loaderDelegate.unloadClass(className);
180-
}
181-
182-
/**
183-
* Returns JShell ClassLoader
184-
*/
185-
public ClassLoader getClassLoader() {
186-
return loaderDelegate.getClassLoader();
187-
}
188-
189-
@Override
190-
public String toString() {
191-
return "JJavaExecutionControl{" +
192-
"timeoutTime=" + timeoutTime +
193-
", timeoutUnit=" + timeoutUnit +
194-
'}';
195-
}
196166
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,10 @@ public ExecutionControl generate(ExecutionEnv env, Map<String, String> parameter
6565
}
6666
}
6767

68+
JJavaLoaderDelegate loaderDelegate = new JJavaLoaderDelegate();
6869
JJavaExecutionControl control = timeout > 0
69-
? new JJavaExecutionControl(timeout, timeUnit)
70-
: new JJavaExecutionControl();
70+
? new JJavaExecutionControl(loaderDelegate, timeout, timeUnit)
71+
: new JJavaExecutionControl(loaderDelegate, -1, TimeUnit.MILLISECONDS);
7172

7273
String id = parameters.get(REGISTRATION_ID_KEY);
7374
if (id != null)

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,29 @@
99
import java.net.URLClassLoader;
1010
import java.nio.file.Path;
1111
import java.security.CodeSource;
12-
import java.util.HashMap;
1312
import java.util.Map;
13+
import java.util.concurrent.ConcurrentHashMap;
1414

1515
public class JJavaLoaderDelegate implements LoaderDelegate {
1616

1717
private static final String CLASSPATH_PROPERTY = "java.class.path";
1818

1919
private final Map<String, byte[]> declaredClasses;
2020
private final Map<String, Class<?>> loadedClasses;
21-
private final BytecodeClassLoader classLoader;
21+
private final JJavaClassLoader classLoader;
2222

2323
public JJavaLoaderDelegate() {
24-
this.declaredClasses = new HashMap<>();
25-
this.loadedClasses = new HashMap<>();
26-
this.classLoader = new BytecodeClassLoader();
24+
this.declaredClasses = new ConcurrentHashMap<>();
25+
this.loadedClasses = new ConcurrentHashMap<>();
26+
this.classLoader = new JJavaClassLoader();
2727
Thread.currentThread().setContextClassLoader(classLoader);
2828
}
2929

3030
@Override
3131
public void load(ExecutionControl.ClassBytecodes[] cbcs) throws ExecutionControl.ClassInstallException {
3232
boolean[] installed = new boolean[cbcs.length];
3333
int i = 0;
34-
for(var cbc: cbcs) {
34+
for (ExecutionControl.ClassBytecodes cbc : cbcs) {
3535
declaredClasses.put(cbc.name(), cbc.bytecodes());
3636
try {
3737
Class<?> loaderClass = classLoader.findClass(cbc.name());
@@ -46,7 +46,7 @@ public void load(ExecutionControl.ClassBytecodes[] cbcs) throws ExecutionControl
4646

4747
@Override
4848
public void classesRedefined(ExecutionControl.ClassBytecodes[] cbcs) {
49-
for(var cbc: cbcs) {
49+
for (ExecutionControl.ClassBytecodes cbc : cbcs) {
5050
declaredClasses.put(cbc.name(), cbc.bytecodes());
5151
}
5252
}
@@ -74,7 +74,7 @@ public Class<?> findClass(String name) throws ClassNotFoundException {
7474
// check if it was removed
7575
klass = loadClass(name);
7676
}
77-
if(klass == null) {
77+
if (klass == null) {
7878
throw new ClassNotFoundException(name + " not found");
7979
}
8080
return klass;
@@ -93,12 +93,13 @@ public ClassLoader getClassLoader() {
9393
return classLoader;
9494
}
9595

96-
class BytecodeClassLoader extends URLClassLoader {
96+
class JJavaClassLoader extends URLClassLoader {
9797

98-
public BytecodeClassLoader() {
98+
public JJavaClassLoader() {
9999
super(new URL[0]);
100100
}
101101

102+
@Override
102103
public void addURL(URL url) {
103104
super.addURL(url);
104105
}

0 commit comments

Comments
 (0)