Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-35098] Disable AutoBrowserHolder by default to improve the c…
…hangelog rendering performance (#2371)

* [FIXED JENKINS-35098] Deleting AutoBrowserHolder.

* Normalizing form binding scheme for AbstractProject.scm.

* At @oleg-nenashev’s insistence, restoring AutoBrowserHolder, though not using it by default.

* Using SystemProperties at @oleg-nenashev’s recommendation.
  • Loading branch information
jglick authored and oleg-nenashev committed Jun 10, 2016
1 parent ceb36b5 commit d33df0f
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 24 deletions.
2 changes: 2 additions & 0 deletions core/src/main/java/hudson/scm/AutoBrowserHolder.java
Expand Up @@ -37,7 +37,9 @@
* *
* <p> * <p>
* This class makes such tracking easy by hiding this logic. * This class makes such tracking easy by hiding this logic.
* @deprecated Disabled by default: JENKINS-35098
*/ */
@Deprecated
final class AutoBrowserHolder { final class AutoBrowserHolder {
private int cacheGeneration; private int cacheGeneration;
private RepositoryBrowser cache; private RepositoryBrowser cache;
Expand Down
11 changes: 5 additions & 6 deletions core/src/main/java/hudson/scm/NullSCM.java
Expand Up @@ -31,16 +31,19 @@
import hudson.model.TaskListener; import hudson.model.TaskListener;
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import net.sf.json.JSONObject;
import org.jenkinsci.Symbol; import org.jenkinsci.Symbol;
import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.DataBoundConstructor;


/** /**
* No {@link SCM}. * No {@link SCM}.
* *
* @author Kohsuke Kawaguchi * @author Kohsuke Kawaguchi
*/ */
public class NullSCM extends SCM { public class NullSCM extends SCM {

@DataBoundConstructor
public NullSCM() {}

@Override public SCMRevisionState calcRevisionsFromBuild(Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { @Override public SCMRevisionState calcRevisionsFromBuild(Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
return null; return null;
} }
Expand Down Expand Up @@ -69,9 +72,5 @@ public DescriptorImpl() {
return Messages.NullSCM_DisplayName(); return Messages.NullSCM_DisplayName();
} }


@Override
public SCM newInstance(StaplerRequest req, JSONObject formData) throws FormException {
return new NullSCM();
}
} }
} }
20 changes: 15 additions & 5 deletions core/src/main/java/hudson/scm/SCM.java
Expand Up @@ -58,6 +58,7 @@
import javax.annotation.Nonnull; import javax.annotation.Nonnull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import jenkins.model.Jenkins; import jenkins.model.Jenkins;
import jenkins.util.SystemProperties;
import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean; import org.kohsuke.stapler.export.ExportedBean;


Expand All @@ -84,8 +85,13 @@
*/ */
@ExportedBean @ExportedBean
public abstract class SCM implements Describable<SCM>, ExtensionPoint { public abstract class SCM implements Describable<SCM>, ExtensionPoint {

/** JENKINS-35098: discouraged */
@SuppressWarnings("FieldMayBeFinal")
private static boolean useAutoBrowserHolder = SystemProperties.getBoolean(SCM.class.getName() + ".useAutoBrowserHolder");
/** /**
* Stores {@link AutoBrowserHolder}. Lazily created. * Stores {@link AutoBrowserHolder}. Lazily created.
* @deprecated Unused by default.
*/ */
private transient AutoBrowserHolder autoBrowserHolder; private transient AutoBrowserHolder autoBrowserHolder;


Expand Down Expand Up @@ -124,17 +130,21 @@ public String getType() {
* Returns the applicable {@link RepositoryBrowser} for files * Returns the applicable {@link RepositoryBrowser} for files
* controlled by this {@link SCM}. * controlled by this {@link SCM}.
* @see #guessBrowser * @see #guessBrowser
* @see SCMDescriptor#isBrowserReusable
*/ */
@SuppressWarnings("deprecation")
@Exported(name="browser") @Exported(name="browser")
public final @CheckForNull RepositoryBrowser<?> getEffectiveBrowser() { public final @CheckForNull RepositoryBrowser<?> getEffectiveBrowser() {
RepositoryBrowser<?> b = getBrowser(); RepositoryBrowser<?> b = getBrowser();
if(b!=null) if(b!=null)
return b; return b;
if(autoBrowserHolder==null) if (useAutoBrowserHolder) {
autoBrowserHolder = new AutoBrowserHolder(this); if (autoBrowserHolder == null) {
return autoBrowserHolder.get(); autoBrowserHolder = new AutoBrowserHolder(this);

}
return autoBrowserHolder.get();
} else {
return guessBrowser();
}
} }


/** /**
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/hudson/scm/SCMDescriptor.java
Expand Up @@ -53,7 +53,9 @@ public abstract class SCMDescriptor<T extends SCM> extends Descriptor<SCM> {
* This is used to invalidate cache of {@link SCM#getEffectiveBrowser}. Due to the lack of synchronization and serialization, * This is used to invalidate cache of {@link SCM#getEffectiveBrowser}. Due to the lack of synchronization and serialization,
* this field doesn't really count the # of instances created to date, * this field doesn't really count the # of instances created to date,
* but it's good enough for the cache invalidation. * but it's good enough for the cache invalidation.
* @deprecated No longer used by default.
*/ */
@Deprecated
public volatile int generation = 1; public volatile int generation = 1;


protected SCMDescriptor(Class<T> clazz, Class<? extends RepositoryBrowser> repositoryBrowser) { protected SCMDescriptor(Class<T> clazz, Class<? extends RepositoryBrowser> repositoryBrowser) {
Expand Down Expand Up @@ -102,7 +104,9 @@ public void load() {
* @return * @return
* true if the two given SCM configurations are similar enough * true if the two given SCM configurations are similar enough
* that they can reuse {@link RepositoryBrowser} between them. * that they can reuse {@link RepositoryBrowser} between them.
* @deprecated No longer used by default. {@link SCM#getKey} could be used to implement similar features if needed.
*/ */
@Deprecated
public boolean isBrowserReusable(T x, T y) { public boolean isBrowserReusable(T x, T y) {
return false; return false;
} }
Expand Down
11 changes: 4 additions & 7 deletions core/src/main/java/hudson/scm/SCMS.java
Expand Up @@ -54,14 +54,11 @@ public class SCMS {
* @param target * @param target
* The project for which this SCM is configured to. * The project for which this SCM is configured to.
*/ */
@SuppressWarnings("deprecation")
public static SCM parseSCM(StaplerRequest req, AbstractProject target) throws FormException, ServletException { public static SCM parseSCM(StaplerRequest req, AbstractProject target) throws FormException, ServletException {
String scm = req.getParameter("scm"); SCM scm = req.bindJSON(SCM.class, req.getSubmittedForm().getJSONObject("scm"));
if(scm==null) return new NullSCM(); scm.getDescriptor().generation++;

This comment has been minimized.

Copy link
@deece

deece Jun 29, 2016

This change introduces a NullPointerException for Multibranch Projects (which use the SCM API plugin).

I suggest that scm be checked for null before attempting to call methods on it.

Stack trace

javax.servlet.ServletException: java.lang.NullPointerException
at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:796)
at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
at org.kohsuke.stapler.MetaClass$5.doDispatch(MetaClass.java:233)
at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
at org.kohsuke.stapler.Stapler.invoke(Stapler.java:649)
at org.kohsuke.stapler.Stapler.service(Stapler.java:238)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:812)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1669)
at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:135)
at net.bull.javamelody.MonitoringFilter.doFilter(MonitoringFilter.java:201)
at net.bull.javamelody.MonitoringFilter.doFilter(MonitoringFilter.java:178)
at net.bull.javamelody.PluginMonitoringFilter.doFilter(PluginMonitoringFilter.java:85)
at org.jvnet.hudson.plugins.monitoring.HudsonMonitoringFilter.doFilter(HudsonMonitoringFilter.java:104)
at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:132)
at org.jenkinsci.plugins.modernstatus.ModernStatusFilter.doFilter(ModernStatusFilter.java:52)
at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:132)
at hudson.plugins.greenballs.GreenBallFilter.doFilter(GreenBallFilter.java:59)
at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:132)
at hudson.util.PluginServletFilter.doFilter(PluginServletFilter.java:126)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
at hudson.security.csrf.CrumbFilter.doFilter(CrumbFilter.java:49)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:84)
at hudson.security.UnwrapSecurityExceptionFilter.doFilter(UnwrapSecurityExceptionFilter.java:51)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
at jenkins.security.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:117)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
at org.acegisecurity.providers.anonymous.AnonymousProcessingFilter.doFilter(AnonymousProcessingFilter.java:125)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
at org.acegisecurity.ui.rememberme.RememberMeProcessingFilter.doFilter(RememberMeProcessingFilter.java:142)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
at org.acegisecurity.ui.AbstractProcessingFilter.doFilter(AbstractProcessingFilter.java:271)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
at jenkins.security.BasicHeaderProcessor.doFilter(BasicHeaderProcessor.java:93)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
at org.acegisecurity.context.HttpSessionContextIntegrationFilter.doFilter(HttpSessionContextIntegrationFilter.java:249)
at hudson.security.HttpSessionContextIntegrationFilter2.doFilter(HttpSessionContextIntegrationFilter2.java:67)
at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
at hudson.security.ChainedServletFilter.doFilter(ChainedServletFilter.java:76)
at hudson.security.HudsonFilter.doFilter(HudsonFilter.java:171)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
at org.kohsuke.stapler.compression.CompressionFilter.doFilter(CompressionFilter.java:49)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
at hudson.util.CharacterEncodingFilter.doFilter(CharacterEncodingFilter.java:82)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
at org.kohsuke.stapler.DiagnosticThreadNameFilter.doFilter(DiagnosticThreadNameFilter.java:30)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:585)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:553)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:223)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
at org.eclipse.jetty.server.Server.handle(Server.java:499)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:311)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257)
at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)
at winstone.BoundedExecutorService$1.run(BoundedExecutorService.java:77)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
at hudson.scm.SCMS.parseSCM(SCMS.java:60)
at hudson.model.AbstractProject.submit(AbstractProject.java:1870)
at hudson.matrix.MatrixProject.submit(MatrixProject.java:890)
at hudson.model.Job.doConfigSubmit(Job.java:1229)
at hudson.model.AbstractProject.doConfigSubmit(AbstractProject.java:796)
at com.github.mjdetullio.jenkins.plugins.multibranch.AbstractMultiBranchProject.submit(AbstractMultiBranchProject.java:539)
at com.cloudbees.hudson.plugins.folder.AbstractFolder.doConfigSubmit(AbstractFolder.java:713)
at com.cloudbees.hudson.plugins.folder.computed.ComputedFolder.doConfigSubmit(ComputedFolder.java:225)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.kohsuke.stapler.Function$InstanceFunction.invoke(Function.java:324)
at org.kohsuke.stapler.interceptor.RequirePOST$Processor.invoke(RequirePOST.java:52)
at org.kohsuke.stapler.PreInvokeInterceptedFunction.invoke(PreInvokeInterceptedFunction.java:26)
at org.kohsuke.stapler.Function.bindAndInvoke(Function.java:167)
at org.kohsuke.stapler.Function.bindAndInvokeAndServeResponse(Function.java:100)
at org.kohsuke.stapler.MetaClass$1.doDispatch(MetaClass.java:124)
at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
... 67 more


return scm;
int scmidx = Integer.parseInt(scm);
SCMDescriptor<?> d = SCM._for(target).get(scmidx);
d.generation++;
return d.newInstance(req, req.getSubmittedForm().getJSONObject("scm"));
} }


/** /**
Expand Down
12 changes: 6 additions & 6 deletions core/src/main/resources/lib/hudson/project/config-scm.jelly
Expand Up @@ -26,14 +26,14 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?> <?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form"> <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<f:section title="${%Source Code Management}"> <f:section title="${%Source Code Management}">
<j:set var="scms" value="${h.getSCMDescriptors(it)}" /> <!-- Could use f:descriptorRadioList were it not for scm/scmd compatibility code; pending JENKINS-20959 would not help with performance -->
<j:forEach var="idx" begin="0" end="${size(scms)-1}"> <j:forEach var="descriptor" items="${h.getSCMDescriptors(instance)}" varStatus="loop">
<j:set var="descriptor" value="${scms[idx]}" />
<j:set var="scmd" value="${descriptor}" /><!-- backward compatibility with <1.238 --> <j:set var="scmd" value="${descriptor}" /><!-- backward compatibility with <1.238 -->
<f:radioBlock name="scm" value="${idx}" title="${scmd.displayName}" checked="${it.scm.descriptor==scmd}"> <f:radioBlock name="scm" help="${descriptor.helpFile}" value="${loop.index}" title="${descriptor.displayName}" checked="${instance.scm.descriptor == descriptor}">
<j:set var="instance" value="${it.scm.descriptor==descriptor ? it.scm : null}"/> <j:set var="instance" value="${instance.scm.descriptor == descriptor ? it.scm : null}"/>
<j:set var="scm" value="${instance}" /><!-- backward compatibility with <1.238 --> <j:set var="scm" value="${instance}" /><!-- backward compatibility with <1.238 -->
<st:include from="${scmd}" page="${scmd.configPage}"/> <f:class-entry descriptor="${descriptor}" />
<st:include from="${descriptor}" page="${descriptor.configPage}" optional="true"/>
</f:radioBlock> </f:radioBlock>
</j:forEach> </j:forEach>
</f:section> </f:section>
Expand Down

0 comments on commit d33df0f

Please sign in to comment.