From 776e004f059d4d4b4d6e3f24490aa0336762f614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Wed, 18 May 2016 08:33:35 -0400 Subject: [PATCH] Synchronize MyersDiff and HistogramDiff invocations on local variable I4516bcc2c synchronized MyersDiff and HistogramDiff invocations but it used a synchronization object that is not local to the PatchList being loaded. In other words, even if PatchList can be loaded in parallel, only one file header diff can be computed at the time across the entire server. In stable-2.12, a fresh instance of PatchListLoaded is used every time a PatchList is loaded so this change is not needed, and must be reverted when merging stable-2.11 into stable-2.12. Change-Id: I5acace093b16c9b87cf465c515018433c3f38d6a --- .../com/google/gerrit/server/patch/PatchListLoader.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java index 13cb50673b..838b3433de 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java @@ -87,7 +87,6 @@ public class PatchListLoader extends CacheLoader { private final ThreeWayMergeStrategy mergeStrategy; private final ExecutorService diffExecutor; private final long timeoutMillis; - private final Object lock; @Inject PatchListLoader(GitRepositoryManager mgr, @@ -98,7 +97,6 @@ public class PatchListLoader extends CacheLoader { patchListCache = plc; mergeStrategy = MergeUtil.getMergeStrategy(cfg); diffExecutor = de; - lock = new Object(); timeoutMillis = ConfigUtil.getTimeUnit(cfg, "cache", PatchListCacheImpl.FILE_NAME, "timeout", TimeUnit.MILLISECONDS.convert(5, TimeUnit.SECONDS), @@ -206,7 +204,7 @@ public class PatchListLoader extends CacheLoader { Future result = diffExecutor.submit(new Callable() { @Override public FileHeader call() throws IOException { - synchronized (lock) { + synchronized (diffEntry) { return diffFormatter.toFileHeader(diffEntry); } } @@ -222,7 +220,7 @@ public class PatchListLoader extends CacheLoader { + " comparing " + diffEntry.getOldId().name() + ".." + diffEntry.getNewId().name()); result.cancel(true); - synchronized (lock) { + synchronized (diffEntry) { return toFileHeaderWithoutMyersDiff(diffFormatter, diffEntry); } } catch (ExecutionException e) {