Skip to content

Commit d0f0248

Browse files
dwnusbaumdwnusbaum
authored
Merge pull request #877 from jglick/GroovyCategorySupport
More efficient use of `GroovyCategorySupport`
2 parents 1a8a6a6 + 9874f12 commit d0f0248

File tree

8 files changed

+86
-65
lines changed

8 files changed

+86
-65
lines changed

‎lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
*/
2424
public class Continuable implements Serializable {
2525

26+
/**
27+
* Users of this library must pass (at least) these to {@link GroovyCategorySupport#use(List, Closure)} during all operations.
28+
*/
2629
@SuppressWarnings("rawtypes")
2730
public static final List<Class> categories = List.of(
2831
CpsDefaultGroovyMethods.class,
@@ -133,31 +136,21 @@ public Object runByThrow(Throwable arg) throws InvocationTargetException {
133136
return run0(new Outcome(null,arg)).wrapReplay();
134137
}
135138

136-
@Deprecated
137-
public Outcome run0(final Outcome cn) {
138-
return run0(cn, categories);
139-
}
140-
141139
/**
142140
* Resumes this program by either returning the value from {@link Continuable#suspend(Object)} or
143141
* throwing an exception
144142
*/
145-
public Outcome run0(final Outcome cn, List<Class> categories) {
146-
return GroovyCategorySupport.use(categories, new Closure<>(null) {
147-
@Override
148-
public Outcome call() {
149-
Next n = cn.resumeFrom(e,k);
150-
151-
while(n.yield==null) {
152-
n = n.step();
153-
}
154-
155-
e = n.e;
156-
k = n.k;
157-
158-
return n.yield;
159-
}
160-
});
143+
public Outcome run0(final Outcome cn) {
144+
Next n = cn.resumeFrom(e,k);
145+
146+
while(n.yield==null) {
147+
n = n.step();
148+
}
149+
150+
e = n.e;
151+
k = n.k;
152+
153+
return n.yield;
161154
}
162155

163156
/**

‎lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,6 @@ public static Iterable<Object[]> generateParameters() {
334334
asList("uniqueSet", "([1, 2, -2, 3] as HashSet).unique { i -> i * i }.collect { it.abs() } as HashSet", asList(1, 2, 3) as HashSet),
335335
*/
336336

337-
// TODO: use?
338-
339337
// TODO: with?
340338

341339
// .withDefault

‎lib/src/test/java/com/cloudbees/groovy/cps/CpsTransformerTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ public void rehydrateClosure() throws Throwable {
769769

770770
@Issue("https://github.com/cloudbees/groovy-cps/issues/16")
771771
@Test
772-
@Ignore
772+
@Ignore("cannot easily be supported")
773773
public void category() throws Throwable {
774774
assertEvaluate("FOO",
775775
"class BarCategory {\n" +

‎lib/src/test/java/com/cloudbees/groovy/cps/impl/NotBlockTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import groovy.lang.Script;
88
import java.io.InputStream;
99
import java.io.ObjectInputStream;
10-
import java.util.Collections;
1110
import org.junit.Test;
1211

1312
import static org.hamcrest.CoreMatchers.equalTo;
@@ -38,6 +37,6 @@ public void serialFormBackwardsCompatibility() throws Throwable {
3837
c = (Continuable)ois.readObject();
3938
}
4039
assertTrue(c.isResumable());
41-
assertThat(c.run0(new Outcome(false, null), Collections.emptyList()).replay(), equalTo(true));
40+
assertThat(c.run0(new Outcome(false, null)).replay(), equalTo(true));
4241
}
4342
}

‎plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import com.cloudbees.groovy.cps.Continuable;
2828
import com.cloudbees.groovy.cps.Outcome;
29-
import com.google.common.collect.ImmutableList;
3029
import com.google.common.util.concurrent.FutureCallback;
3130
import java.io.IOException;
3231
import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
@@ -43,7 +42,6 @@
4342
import java.util.concurrent.TimeUnit;
4443
import java.util.logging.Logger;
4544

46-
import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;
4745

4846
import static java.util.logging.Level.FINE;
4947
import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.PROGRAM;
@@ -162,11 +160,6 @@ public StepExecution getStep() {
162160
this.step = step;
163161
}
164162

165-
private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
166-
.addAll(Continuable.categories)
167-
.add(IteratorHack.class)
168-
.build();
169-
170163
/**
171164
* Executes CPS code synchronously a little bit more, until it hits
172165
* the point the workflow needs to be dehydrated.
@@ -184,7 +177,7 @@ public StepExecution getStep() {
184177
LOGGER.fine(() -> "runNextChunk on " + resumeValue);
185178
final Outcome o = resumeValue;
186179
resumeValue = null;
187-
outcome = program.run0(o, CATEGORIES);
180+
outcome = program.run0(o);
188181
if (outcome.getAbnormal() != null) {
189182
LOGGER.log(FINE, "ran and produced error", outcome.getAbnormal());
190183
} else {

‎plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java‎

Lines changed: 65 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
package org.jenkinsci.plugins.workflow.cps;
22

3+
import com.cloudbees.groovy.cps.Continuable;
34
import com.cloudbees.groovy.cps.impl.CpsCallableInvocation;
5+
import com.google.common.collect.ImmutableList;
46
import hudson.Main;
5-
import hudson.model.Computer;
67
import hudson.remoting.SingleLaneExecutorService;
78
import hudson.security.ACL;
89
import java.io.IOException;
@@ -11,9 +12,21 @@
1112
import java.util.logging.Level;
1213
import java.util.logging.Logger;
1314
import edu.umd.cs.findbugs.annotations.CheckForNull;
15+
import groovy.lang.Closure;
1416
import hudson.model.TaskListener;
17+
import hudson.util.DaemonThreadFactory;
18+
import hudson.util.ExceptionCatchingThreadFactory;
19+
import hudson.util.NamingThreadFactory;
20+
import java.util.List;
21+
import java.util.concurrent.Executors;
22+
import java.util.concurrent.ThreadFactory;
1523
import jenkins.model.Jenkins;
24+
import jenkins.security.ImpersonatingExecutorService;
25+
import jenkins.util.ContextResettingExecutorService;
26+
import jenkins.util.ErrorLoggingExecutorService;
1627
import jenkins.util.InterceptingExecutorService;
28+
import org.codehaus.groovy.runtime.GroovyCategorySupport;
29+
import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;
1730
import org.jenkinsci.plugins.workflow.graph.FlowNode;
1831
import org.jenkinsci.plugins.workflow.steps.StepExecution;
1932

@@ -24,27 +37,55 @@
2437
* @see CpsVmThreadOnly
2538
*/
2639
class CpsVmExecutorService extends InterceptingExecutorService {
40+
41+
@SuppressWarnings("rawtypes")
42+
private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
43+
.addAll(Continuable.categories)
44+
.add(IteratorHack.class)
45+
.build();
46+
47+
private static ThreadFactory categoryThreadFactory(ThreadFactory core) {
48+
return r -> core.newThread(() -> {
49+
LOGGER.fine("spawning new thread");
50+
GroovyCategorySupport.use(CATEGORIES, new Closure<Void>(null) {
51+
@Override
52+
public Void call() {
53+
r.run();
54+
return null;
55+
}
56+
});
57+
});
58+
}
59+
60+
private static final ExecutorService threadPool = new ContextResettingExecutorService(
61+
new ImpersonatingExecutorService(
62+
new ErrorLoggingExecutorService(
63+
Executors.newCachedThreadPool(
64+
categoryThreadFactory(
65+
new ExceptionCatchingThreadFactory(
66+
new NamingThreadFactory(
67+
new DaemonThreadFactory(),
68+
"CpsVmExecutorService"))))),
69+
ACL.SYSTEM2));
70+
2771
private CpsThreadGroup cpsThreadGroup;
2872

29-
public CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
30-
super(new SingleLaneExecutorService(Computer.threadPoolForRemoting));
73+
CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
74+
super(new SingleLaneExecutorService(threadPool));
3175
this.cpsThreadGroup = cpsThreadGroup;
3276
}
3377

3478
@Override
3579
protected Runnable wrap(final Runnable r) {
36-
return new Runnable() {
37-
@Override
38-
public void run() {
39-
ThreadContext context = setUp();
40-
try {
41-
r.run();
42-
} catch (final Throwable t) {
43-
reportProblem(t);
44-
throw t;
45-
} finally {
46-
tearDown(context);
47-
}
80+
return () -> {
81+
ThreadContext context = setUp();
82+
try {
83+
r.run();
84+
} catch (final Throwable t) {
85+
reportProblem(t);
86+
throw t;
87+
} finally {
88+
tearDown(context);
4889
}
4990
};
5091
}
@@ -89,18 +130,15 @@ private void reportProblem(Throwable t) {
89130

90131
@Override
91132
protected <V> Callable<V> wrap(final Callable<V> r) {
92-
return new Callable<>() {
93-
@Override
94-
public V call() throws Exception {
95-
ThreadContext context = setUp();
96-
try {
97-
return r.call();
98-
} catch (final Throwable t) {
99-
reportProblem(t);
100-
throw t;
101-
} finally {
102-
tearDown(context);
103-
}
133+
return () -> {
134+
ThreadContext context = setUp();
135+
try {
136+
return r.call();
137+
} catch (final Throwable t) {
138+
reportProblem(t);
139+
throw t;
140+
} finally {
141+
tearDown(context);
104142
}
105143
};
106144
}

‎plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/SandboxContinuable.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import hudson.console.ConsoleAnnotator;
88
import hudson.console.ConsoleNote;
99
import java.io.IOException;
10-
import java.util.List;
1110
import java.util.logging.Level;
1211
import java.util.logging.Logger;
1312
import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox;
@@ -24,8 +23,7 @@ class SandboxContinuable extends Continuable {
2423
}
2524

2625
@SuppressWarnings("rawtypes")
27-
@Override
28-
public Outcome run0(final Outcome cn, final List<Class> categories) {
26+
public Outcome run0(final Outcome cn) {
2927
CpsFlowExecution e = thread.group.getExecution();
3028
if (e == null) {
3129
throw new IllegalStateException("JENKINS-50407: no loaded execution");
@@ -48,7 +46,7 @@ public Outcome run0(final Outcome cn, final List<Class> categories) {
4846
trustedShell.getClassLoader(),
4947
shell.getClassLoader()));
5048
try (GroovySandbox.Scope scope = sandbox.enter()) {
51-
return SandboxContinuable.super.run0(cn, categories);
49+
return SandboxContinuable.super.run0(cn);
5250
}
5351
}
5452

‎plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionMemoryTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
package org.jenkinsci.plugins.workflow.cps;
2626

2727
import groovy.lang.MetaClass;
28+
import hudson.model.Computer;
2829
import java.lang.ref.WeakReference;
2930
import java.lang.reflect.Field;
3031
import java.lang.reflect.Method;
@@ -67,6 +68,7 @@ public static void register(Object o) {
6768
}
6869

6970
@Test public void loaderReleased() throws Exception {
71+
Computer.threadPoolForRemoting.submit(() -> {}).get(); // do not let it be initialized inside the CpsVmExecutorService thread group
7072
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
7173
r.jenkins.getWorkspaceFor(p).child("lib.groovy").write(CpsFlowExecutionMemoryTest.class.getName() + ".register(this)", null);
7274
p.setDefinition(new CpsFlowDefinition(CpsFlowExecutionMemoryTest.class.getName() + ".register(this); node {load 'lib.groovy'; evaluate(readFile('lib.groovy'))}", false));

0 commit comments

Comments
 (0)