changeset 600:6c480513e5c2

Optimize subrepo changes calculation: reuse subrepo revision table calculated for parent commit
author Dmitry Neverov <dmitry.neverov@jetbrains.com>
date Thu, 30 May 2013 15:09:10 +0400
parents 109d7d3cdc8f
children d2d9e06ec5d7
files mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgRepo.java mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialCollectChangesPolicy.java mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/RepoFactory.java mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/ServerHgRepo.java mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialSupportBuilder.java mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/SubrepoChangesTest.java
diffstat 6 files changed, 134 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgRepo.java	Wed May 22 12:35:08 2013 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgRepo.java	Thu May 30 15:09:10 2013 +0400
@@ -25,7 +25,7 @@
   protected final File myWorkingDir;
   protected final String myHgPath;
   protected final AuthSettings myAuthSettings;
-  private final Map<String, Map<String, SubRepo>> mySubreposCache = new HashMap<String, Map<String, SubRepo>>();
+  protected final Map<String, Map<String, SubRepo>> mySubreposCache = new HashMap<String, Map<String, SubRepo>>();
 
   public HgRepo(@NotNull CommandSettingsFactory commandSettingsFactory,
                 @NotNull File workingDir,
--- a/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialCollectChangesPolicy.java	Wed May 22 12:35:08 2013 +0400
+++ b/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialCollectChangesPolicy.java	Thu May 30 15:09:10 2013 +0400
@@ -17,6 +17,8 @@
 
 public class MercurialCollectChangesPolicy implements CollectChangesBetweenRoots, CollectChangesBetweenRepositories {
 
+  private final static AscendingRevNums ASCENDING_REV_NUMS = new AscendingRevNums();
+
   private final MercurialVcsSupport myVcs;
   private final ServerPluginConfig myConfig;
   private final HgVcsRootFactory myHgVcsRootFactory;
@@ -176,7 +178,12 @@
     HgVcsRoot hgRoot = myHgVcsRootFactory.createHgRoot(root);
     ctx.syncRepository(hgRoot);
     List<ModificationData> result = new ArrayList<ModificationData>();
-    for (ChangeSet cset : getChangesets(ctx, hgRoot, fromVersion, currentVersion)) {
+    List<ChangeSet> csets = getChangesets(ctx, hgRoot, fromVersion, currentVersion);
+    //When commit has no changes in subrepo configuration we can reuse
+    //subrepo revision table calculated for its parent commit(s). To do
+    //that parents should be processed before children:
+    Collections.sort(csets, ASCENDING_REV_NUMS);
+    for (ChangeSet cset : csets) {
       result.add(createModificationData(ctx, cset, root, checkoutRules));
     }
     return result;
@@ -233,7 +240,7 @@
       result.addParentRevision(ctx.getStringFromPool(parent.getId()));
     }
     setCanBeIgnored(result, cset);
-    result.setAttributes(getAttributes(ctx, root, cset));
+    result.setAttributes(getAttributes(ctx, root, result));
     return result;
   }
 
@@ -358,7 +365,7 @@
 
     HgVcsRoot mainRoot = myHgVcsRootFactory.createHgRoot(m.getVcsRoot());
     ServerHgRepo repo = myVcs.createRepo(ctx, mainRoot);
-    for (HgSubrepoConfigChange c : repo.getSubrepoConfigChanges(m.getVersion(), m.getParentRevisions())) {
+    for (HgSubrepoConfigChange c : repo.getSubrepoConfigChanges(m)) {
       if (!(c.subrepoUrlChanged() || c.subrepoAdded() || c.subrepoRemoved())) {//report only changes in revisions, because we collect changes only for such changes
         //map url and path, relative to the main repository
         try {
@@ -382,14 +389,14 @@
 
 
   @NotNull
-  private Map<String, String> getAttributes(@NotNull OperationContext ctx, @NotNull VcsRoot mainRoot, @NotNull ChangeSet cset) throws VcsException {
+  private Map<String, String> getAttributes(@NotNull OperationContext ctx, @NotNull VcsRoot mainRoot, @NotNull ModificationData m) throws VcsException {
     Map<String, String> attributes = new HashMap<String, String>();
     HgVcsRoot root = myHgVcsRootFactory.createHgRoot(mainRoot);
     if (detectSubrepoChanges(root)) {
       try {
         ServerHgRepo repo = myVcs.createRepo(ctx, root);
         SubrepoRevisionAttributesBuilder attrBuilder = new SubrepoRevisionAttributesBuilder();
-        for (SubRepo s : repo.getSubrepositories(cset).values()) {
+        for (SubRepo s : repo.getSubrepositories(m).values()) {
           attrBuilder.addSubrepo(new SubrepoConfig(root)
                   .setSubrepoPath(ctx.getStringFromPool(root.expandSubrepoPath(s.path())))
                   .setSubrepoRootParamDiff(Constants.REPOSITORY_PROP, ctx.getStringFromPool(s.resolveUrl(root.getRepository())))
@@ -409,4 +416,16 @@
       return emptyMap();
     return attributes;
   }
+
+  private final static class AscendingRevNums implements Comparator<ChangeSet> {
+    public int compare(ChangeSet o1, ChangeSet o2) {
+      int revnum1 = o1.getRevNumber();
+      int revnum2 = o2.getRevNumber();
+      if (revnum1 < revnum2)
+        return -1;
+      if (revnum1 > revnum2)
+        return 1;
+      return 0;
+    }
+  }
 }
--- a/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/RepoFactory.java	Wed May 22 12:35:08 2013 +0400
+++ b/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/RepoFactory.java	Thu May 30 15:09:10 2013 +0400
@@ -15,15 +15,15 @@
 /**
  * @author dmitry.neverov
  */
-public final class RepoFactory {
+public class RepoFactory {
 
-  private final ServerPluginConfig myConfig;
-  private final CommandSettingsFactory myCommandSettingsFactory;
+  protected final ServerPluginConfig myConfig;
+  protected final CommandSettingsFactory myCommandSettingsFactory;
 
-  private final MercurialLogTemplate myLogTemplate = new MercurialLogTemplate("/buildServerResources/log.template", "hg.log.template");
-  private final MercurialLogTemplate myLogNoFilesTemplate = new MercurialLogTemplate("/buildServerResources/log.no.files.template", "hg.short.log.template");
-  private final MercurialLogTemplate myDagTemplate = new MercurialLogTemplate("/buildServerResources/dag.template", "hg.dag.template");
-  private final MercurialLogTemplate myFastLogTemplate = new MercurialLogTemplate("/buildServerResources/fastlog.template", "hg.fastlog.template");
+  protected final MercurialLogTemplate myLogTemplate = new MercurialLogTemplate("/buildServerResources/log.template", "hg.log.template");
+  protected final MercurialLogTemplate myLogNoFilesTemplate = new MercurialLogTemplate("/buildServerResources/log.no.files.template", "hg.short.log.template");
+  protected final MercurialLogTemplate myDagTemplate = new MercurialLogTemplate("/buildServerResources/dag.template", "hg.dag.template");
+  protected final MercurialLogTemplate myFastLogTemplate = new MercurialLogTemplate("/buildServerResources/fastlog.template", "hg.fastlog.template");
 
   public RepoFactory(@NotNull ServerPluginConfig config, @NotNull CommandSettingsFactory commandSettingsFactory) throws IOException {
     myConfig = config;
@@ -48,7 +48,7 @@
     myFastLogTemplate.dispose();
   }
 
-  private static class MercurialLogTemplate {
+  static class MercurialLogTemplate {
     private final String myResourcePath;
     private final String myTmpFileSuffix;
     private File myFile;
--- a/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/ServerHgRepo.java	Wed May 22 12:35:08 2013 +0400
+++ b/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/ServerHgRepo.java	Thu May 30 15:09:10 2013 +0400
@@ -4,11 +4,16 @@
 import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.*;
 import jetbrains.buildServer.util.graph.DAG;
 import jetbrains.buildServer.util.graph.DAGs;
+import jetbrains.buildServer.vcs.ModificationData;
+import jetbrains.buildServer.vcs.VcsChange;
 import jetbrains.buildServer.vcs.VcsException;
 import org.jetbrains.annotations.NotNull;
 
 import java.io.File;
 import java.util.List;
+import java.util.Map;
+
+import static java.util.Collections.emptyList;
 
 /**
  * @author dmitry.neverov
@@ -97,4 +102,32 @@
     List<Pair<String, String>> edges = loadDag.call();
     return DAGs.createFromEdges(edges);
   }
+
+  public Map<String, SubRepo> getSubrepositories(@NotNull ModificationData m) {
+    if (hasSubrepoConfigChanges(m))
+      return getSubrepositories(m.getVersion());
+    for (String p : m.getParentRevisions()) {
+      Map<String, SubRepo> cached = mySubreposCache.get(p);
+      if (cached != null) {
+        mySubreposCache.put(m.getVersion(), cached);
+        return cached;
+      }
+    }
+    return getSubrepositories(m.getVersion());
+  }
+
+  public List<HgSubrepoConfigChange> getSubrepoConfigChanges(@NotNull ModificationData m) {
+    if (hasSubrepoConfigChanges(m))
+      return getSubrepoConfigChanges(m.getVersion(), m.getParentRevisions());
+    return emptyList();
+  }
+
+  private boolean hasSubrepoConfigChanges(@NotNull ModificationData m) {
+    for (VcsChange c : m.getChanges()) {
+      //use endsWith instead of equals because files are mapped by checkout rules
+      if (c.getFileName().endsWith(".hgsub") || c.getFileName().endsWith(".hgsubstate"))
+        return true;
+    }
+    return false;
+  }
 }
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialSupportBuilder.java	Wed May 22 12:35:08 2013 +0400
+++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialSupportBuilder.java	Thu May 30 15:09:10 2013 +0400
@@ -1,15 +1,11 @@
 package jetbrains.buildServer.buildTriggers.vcs.mercurial;
 
-import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandSettingsFactory;
 import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.TestCommandSettingsFactory;
-import jetbrains.buildServer.serverSide.BuildServerListener;
-import jetbrains.buildServer.serverSide.SBuildServer;
 import jetbrains.buildServer.serverSide.ServerListener;
 import jetbrains.buildServer.util.EventDispatcher;
 import jetbrains.buildServer.util.cache.ResetCacheHandler;
 import jetbrains.buildServer.util.cache.ResetCacheRegister;
 import jetbrains.buildServer.vcs.SubrepoCheckoutRulesProviderImpl;
-import jetbrains.buildServer.vcs.VcsManager;
 import org.jetbrains.annotations.NotNull;
 import org.jmock.Expectations;
 import org.jmock.Mockery;
@@ -17,8 +13,6 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
 
 public class MercurialSupportBuilder {
 
@@ -26,7 +20,7 @@
   private ServerPluginConfig myConfig;
   private List<MercurialServerExtension> myExtensions = new ArrayList<MercurialServerExtension>();
   private HgVcsRootFactory myHgRootFactory;
-  private CommandSettingsFactory myCommandSettingsFactory = new TestCommandSettingsFactory();
+  private RepoFactory myRepoFactory;
 
   public static MercurialSupportBuilder mercurialSupport() {
     return new MercurialSupportBuilder();
@@ -40,7 +34,7 @@
     myHgRootFactory = new HgVcsRootFactory(myConfig);
     MirrorManagerImpl mirrorManager = new MirrorManagerImpl(myConfig);
     ServerHgPathProvider hgPathProvider = new ServerHgPathProvider(myConfig);
-    RepoFactory repoFactory = new RepoFactory(myConfig, myCommandSettingsFactory);
+    RepoFactory repoFactory = myRepoFactory != null ? myRepoFactory : new RepoFactory(myConfig, new TestCommandSettingsFactory());
     HgTestConnectionSupport testConnection = new HgTestConnectionSupport(myHgRootFactory, repoFactory, mirrorManager, hgPathProvider);
     final ResetCacheRegister resetCacheManager = myContext.mock(ResetCacheRegister.class);
     myContext.checking(new Expectations() {{
@@ -58,12 +52,8 @@
     return this;
   }
 
-  public MercurialSupportBuilder withExtension(@NotNull MercurialServerExtension extension) {
-    myExtensions.add(extension);
+  public MercurialSupportBuilder withRepoFactory(@NotNull RepoFactory repoFactory) {
+    myRepoFactory = repoFactory;
     return this;
   }
-
-  public HgVcsRootFactory getHgRootFactory() {
-    return myHgRootFactory;
-  }
 }
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/SubrepoChangesTest.java	Wed May 22 12:35:08 2013 +0400
+++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/SubrepoChangesTest.java	Thu May 30 15:09:10 2013 +0400
@@ -2,6 +2,10 @@
 
 import com.intellij.openapi.diagnostic.Logger;
 import jetbrains.buildServer.TempFiles;
+import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.AuthSettings;
+import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CatCommand;
+import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandSettingsFactory;
+import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.TestCommandSettingsFactory;
 import jetbrains.buildServer.log.Log4jFactory;
 import jetbrains.buildServer.vcs.*;
 import org.jetbrains.annotations.NotNull;
@@ -14,8 +18,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
 
-import static java.util.Arrays.asList;
 import static jetbrains.buildServer.buildTriggers.vcs.mercurial.MercurialSupportBuilder.mercurialSupport;
 import static jetbrains.buildServer.buildTriggers.vcs.mercurial.ModificationDataMatcher.modificationData;
 import static jetbrains.buildServer.buildTriggers.vcs.mercurial.Util.copyRepository;
@@ -37,6 +41,7 @@
   private File myRemoteRepo1;
   private File myRemoteRepo2;
   private File myRemoteRepo3;
+  private File myRepoNoSubs;
 
   @BeforeMethod
   public void setUp() throws IOException {
@@ -49,9 +54,11 @@
     myRemoteRepo1 = new File(remoteRepoParentDir, "r1");
     myRemoteRepo2 = new File(remoteRepoParentDir, "r2");
     myRemoteRepo3 = new File(remoteRepoParentDir, "r3");
+    myRepoNoSubs = new File(remoteRepoParentDir, "repo.no.subs");
     copyRepository(new File("mercurial-tests/testData/subrepos/r1"), myRemoteRepo1);
     copyRepository(new File("mercurial-tests/testData/subrepos/r2"), myRemoteRepo2);
     copyRepository(new File("mercurial-tests/testData/subrepos/r3"), myRemoteRepo3);
+    copyRepository(new File("mercurial-tests/testData/rep1"), myRepoNoSubs);
     myVcs = mercurialSupport().withConfig(pluginConfig).build();
   }
 
@@ -203,4 +210,60 @@
             .setSubrepoRevision("ac0003deae69"));
     assertThat(changes, hasItem(modificationData().withVersion("514c3e09cddf").withAttributes(builder.buildAttributes())));
   }
+
+  public void should_retrieve_subrepo_config_only_when_necessary(@NotNull HgVersion _) throws Exception {
+    //Every commit from a main repository has a table of subrepo revisions.
+    //To build this table we call hg cat .hgsub .hgsubstate. -r <main root revision>
+    //When main repository has a lot of new changes (thousands) calling 'hg cat'
+    //for each main root revision becomes a bottleneck. We should call hg cat only
+    //if a main root change has .hgsub/.hgsubstate among its changed files,
+    //and reuse subrepo table from a parent commit if it doesn't.
+
+    ServerPluginConfig pluginConfig = new ServerPluginConfigBuilder()
+            .cachesDir(myTempFiles.createTempDir())
+            .detectSubrepoChanges(true)
+            .dontUseRevsets()
+            .build();
+
+    final AtomicInteger catCallCounter = new AtomicInteger(0);
+    RepoFactory repoFactory = new RepoFactory(pluginConfig, new TestCommandSettingsFactory()) {
+      @NotNull
+      @Override
+      public ServerHgRepo create(@NotNull File workingDir, @NotNull String hgPath, @NotNull AuthSettings authSettings) throws VcsException {
+        return new CountingServerHgRepo(myCommandSettingsFactory, myConfig, workingDir, hgPath, authSettings, catCallCounter)
+                .withLogTemplates(myLogTemplate.getTemplate(),
+                        myLogNoFilesTemplate.getTemplate(),
+                        myDagTemplate.getTemplate(),
+                        myFastLogTemplate.getTemplate());
+      }
+    };
+
+    myVcs = mercurialSupport().withConfig(pluginConfig).withRepoFactory(repoFactory).build();
+    VcsRoot root = vcsRoot().withUrl(myRepoNoSubs.getAbsolutePath()).withSubrepoChanges(true).build();
+    myVcs.getCollectChangesPolicy().collectChanges(root, "1d446e82d356", "e6935c9c80bf", CheckoutRules.DEFAULT);
+
+    //this root contains no subrepos, we should call getSubrepos just once and then reuse this info
+    assertEquals(1, catCallCounter.intValue());
+  }
+
+
+  protected static class CountingServerHgRepo extends ServerHgRepo {
+    private final AtomicInteger myCatCallCounter;
+
+    public CountingServerHgRepo(@NotNull CommandSettingsFactory commandSettingsFactory,
+                                @NotNull ServerPluginConfig config,
+                                @NotNull File workingDir,
+                                @NotNull String hgPath,
+                                @NotNull AuthSettings authSettings,
+                                @NotNull AtomicInteger catCallCounter) {
+      super(commandSettingsFactory, config, workingDir, hgPath, authSettings);
+      myCatCallCounter = catCallCounter;
+    }
+
+    @Override
+    public CatCommand cat() {
+      myCatCallCounter.incrementAndGet();
+      return super.cat();
+    }
+  }
 }