Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More efficient use of GroovyCategorySupport #877

Merged
merged 2 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 14 additions & 21 deletions lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
*/
public class Continuable implements Serializable {

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

@Deprecated
public Outcome run0(final Outcome cn) {
return run0(cn, categories);
}

/**
* Resumes this program by either returning the value from {@link Continuable#suspend(Object)} or
* throwing an exception
*/
public Outcome run0(final Outcome cn, List<Class> categories) {
return GroovyCategorySupport.use(categories, new Closure<>(null) {
@Override
public Outcome call() {
Next n = cn.resumeFrom(e,k);

while(n.yield==null) {
n = n.step();
}

e = n.e;
k = n.k;

return n.yield;
}
});
public Outcome run0(final Outcome cn) {
Next n = cn.resumeFrom(e,k);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ignore WS)


while(n.yield==null) {
n = n.step();
}

e = n.e;
k = n.k;

return n.yield;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,7 @@
asList("uniqueSet", "([1, 2, -2, 3] as HashSet).unique { i -> i * i }.collect { it.abs() } as HashSet", asList(1, 2, 3) as HashSet),
*/

// TODO: use?

// TODO: with?

Check warning on line 337 in lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: with?

// .withDefault
asList("withDefaultList", "[].withDefault { i -> i * 2 }.get(1)", 2),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ public void rehydrateClosure() throws Throwable {

@Issue("https://github.com/cloudbees/groovy-cps/issues/16")
@Test
@Ignore
@Ignore("cannot easily be supported")
public void category() throws Throwable {
assertEvaluate("FOO",
"class BarCategory {\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import groovy.lang.Script;
import java.io.InputStream;
import java.io.ObjectInputStream;
import java.util.Collections;
import org.junit.Test;

import static org.hamcrest.CoreMatchers.equalTo;
Expand Down Expand Up @@ -38,6 +37,6 @@ public void serialFormBackwardsCompatibility() throws Throwable {
c = (Continuable)ois.readObject();
}
assertTrue(c.isResumable());
assertThat(c.run0(new Outcome(false, null), Collections.emptyList()).replay(), equalTo(true));
assertThat(c.run0(new Outcome(false, null)).replay(), equalTo(true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

import com.cloudbees.groovy.cps.Continuable;
import com.cloudbees.groovy.cps.Outcome;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.FutureCallback;
import java.io.IOException;
import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
Expand All @@ -43,7 +42,6 @@
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;

import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;

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

private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
.addAll(Continuable.categories)
.add(IteratorHack.class)
.build();

/**
* Executes CPS code synchronously a little bit more, until it hits
* the point the workflow needs to be dehydrated.
Expand All @@ -184,7 +177,7 @@ public StepExecution getStep() {
LOGGER.fine(() -> "runNextChunk on " + resumeValue);
final Outcome o = resumeValue;
resumeValue = null;
outcome = program.run0(o, CATEGORIES);
outcome = program.run0(o);
if (outcome.getAbnormal() != null) {
LOGGER.log(FINE, "ran and produced error", outcome.getAbnormal());
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package org.jenkinsci.plugins.workflow.cps;

import com.cloudbees.groovy.cps.Continuable;
import com.cloudbees.groovy.cps.impl.CpsCallableInvocation;
import com.google.common.collect.ImmutableList;
import hudson.Main;
import hudson.model.Computer;
import hudson.remoting.SingleLaneExecutorService;
import hudson.security.ACL;
import java.io.IOException;
Expand All @@ -11,9 +12,21 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import groovy.lang.Closure;
import hudson.model.TaskListener;
import hudson.util.DaemonThreadFactory;
import hudson.util.ExceptionCatchingThreadFactory;
import hudson.util.NamingThreadFactory;
import java.util.List;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
import jenkins.model.Jenkins;
import jenkins.security.ImpersonatingExecutorService;
import jenkins.util.ContextResettingExecutorService;
import jenkins.util.ErrorLoggingExecutorService;
import jenkins.util.InterceptingExecutorService;
import org.codehaus.groovy.runtime.GroovyCategorySupport;
import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.steps.StepExecution;

Expand All @@ -24,27 +37,55 @@
* @see CpsVmThreadOnly
*/
class CpsVmExecutorService extends InterceptingExecutorService {

@SuppressWarnings("rawtypes")
private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
.addAll(Continuable.categories)
.add(IteratorHack.class)
.build();

private static ThreadFactory categoryThreadFactory(ThreadFactory core) {
return r -> core.newThread(() -> {
LOGGER.fine("spawning new thread");
GroovyCategorySupport.use(CATEGORIES, new Closure<Void>(null) {
@Override
public Void call() {
r.run();
return null;
}
});
});
}

private static final ExecutorService threadPool = new ContextResettingExecutorService(
new ImpersonatingExecutorService(
new ErrorLoggingExecutorService(
Executors.newCachedThreadPool(
categoryThreadFactory(
new ExceptionCatchingThreadFactory(
new NamingThreadFactory(
new DaemonThreadFactory(),
"CpsVmExecutorService"))))),
ACL.SYSTEM2));

private CpsThreadGroup cpsThreadGroup;

public CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
super(new SingleLaneExecutorService(Computer.threadPoolForRemoting));
CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
super(new SingleLaneExecutorService(threadPool));
this.cpsThreadGroup = cpsThreadGroup;
}

@Override
protected Runnable wrap(final Runnable r) {
return new Runnable() {
@Override
public void run() {
ThreadContext context = setUp();
try {
r.run();
} catch (final Throwable t) {
reportProblem(t);
throw t;
} finally {
tearDown(context);
}
return () -> {
ThreadContext context = setUp();
try {
r.run();
} catch (final Throwable t) {
reportProblem(t);
throw t;
} finally {
tearDown(context);
}
};
}
Expand Down Expand Up @@ -89,18 +130,15 @@ private void reportProblem(Throwable t) {

@Override
protected <V> Callable<V> wrap(final Callable<V> r) {
return new Callable<>() {
@Override
public V call() throws Exception {
ThreadContext context = setUp();
try {
return r.call();
} catch (final Throwable t) {
reportProblem(t);
throw t;
} finally {
tearDown(context);
}
return () -> {
ThreadContext context = setUp();
try {
return r.call();
} catch (final Throwable t) {
reportProblem(t);
throw t;
} finally {
tearDown(context);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import hudson.console.ConsoleAnnotator;
import hudson.console.ConsoleNote;
import java.io.IOException;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox;
Expand All @@ -24,8 +23,7 @@ class SandboxContinuable extends Continuable {
}

@SuppressWarnings("rawtypes")
@Override
public Outcome run0(final Outcome cn, final List<Class> categories) {
public Outcome run0(final Outcome cn) {
CpsFlowExecution e = thread.group.getExecution();
if (e == null) {
throw new IllegalStateException("JENKINS-50407: no loaded execution");
Expand All @@ -48,7 +46,7 @@ public Outcome run0(final Outcome cn, final List<Class> categories) {
trustedShell.getClassLoader(),
shell.getClassLoader()));
try (GroovySandbox.Scope scope = sandbox.enter()) {
return SandboxContinuable.super.run0(cn, categories);
return SandboxContinuable.super.run0(cn);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
package org.jenkinsci.plugins.workflow.cps;

import groovy.lang.MetaClass;
import hudson.model.Computer;
import java.lang.ref.WeakReference;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -67,6 +68,7 @@ public static void register(Object o) {
}

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