opendev/gerrit
Code Issues Proposed changes

Stop leaking data in PatchLineCache keys

Browse Source 
As in other persisted caches, references to excess data may stick
around longer than we need them. Particularly in the case of the
intraline diff cache, this can be a significant amount of data. Avoid
this by using non-LoadingCaches. Because these loader implementations
are somewhat complex, we need additional factories and args classes.

Change-Id: Ic5a0543f0b70dbeee7dac47bbbd6f4a4fce0815c
This commit is contained in:
Dave Borowitz
2015-08-06 15:57:13 -07:00
committed by Shawn Pearce
parent 9efcff3124
commit fdbb76233c
14 changed files with 172 additions and 159 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java
View File

@@ -236,16 +236,19 @@ public class PatchListCacheIT extends AbstractDaemonTest {
private List<PatchListEntry> getCurrentPatches(String changeId)
throws PatchListNotAvailableException, RestApiException {
return patchListCache.get(getKey(null, getCurrentRevisionId(changeId))).getPatches();
return patchListCache
.get(getKey(null, getCurrentRevisionId(changeId)), project)
.getPatches();
}
private List<PatchListEntry> getPatches(ObjectId revisionIdA, ObjectId revisionIdB)
throws PatchListNotAvailableException {
return patchListCache.get(getKey(revisionIdA, revisionIdB)).getPatches();
return patchListCache.get(getKey(revisionIdA, revisionIdB), project)
.getPatches();
}
private PatchListKey getKey(ObjectId revisionIdA, ObjectId revisionIdB) {
return new PatchListKey(project, revisionIdA, revisionIdB, Whitespace.IGNORE_NONE);
return new PatchListKey(revisionIdA, revisionIdB, Whitespace.IGNORE_NONE);
}
private ObjectId getCurrentRevisionId(String changeId) throws RestApiException {

12
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
View File

@@ -91,7 +91,7 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
private final PatchLineCommentsUtil plcUtil;
private final ChangeEditUtil editUtil;
private Project.NameKey projectKey;
private Project.NameKey project;
private final PatchSet.Id psIdBase;
private final PatchSet.Id psIdNew;
private final AccountDiffPreference diffPrefs;
@@ -150,7 +150,7 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
throw new NoSuchEntityException();
}
}
projectKey = control.getProject().getNameKey();
project = control.getProject().getNameKey();
final PatchList list;
try {
@@ -188,7 +188,7 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
detail = new PatchSetDetail();
detail.setPatchSet(patchSet);
detail.setProject(projectKey);
detail.setProject(project);
detail.setInfo(infoFactory.get(db, patchSet.getId()));
detail.setPatches(patches);
@@ -251,12 +251,12 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
}
}
private PatchListKey keyFor(final Whitespace whitespace) {
return new PatchListKey(projectKey, oldId, newId, whitespace);
private PatchListKey keyFor(Whitespace whitespace) {
return new PatchListKey(oldId, newId, whitespace);
}
private PatchList listFor(PatchListKey key)
throws PatchListNotAvailableException {
return patchListCache.get(key);
return patchListCache.get(key, project);
}
}

6
gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
View File

@@ -99,14 +99,14 @@ public final class StoredValues {
PatchSet ps = getPatchSet(engine);
PatchListCache plCache = env.getArgs().getPatchListCache();
Change change = getChange(engine);
Project.NameKey projectKey = change.getProject();
Project.NameKey project = change.getProject();
ObjectId a = null;
ObjectId b = ObjectId.fromString(ps.getRevision().get());
Whitespace ws = Whitespace.IGNORE_NONE;
PatchListKey plKey = new PatchListKey(projectKey, a, b, ws);
PatchListKey plKey = new PatchListKey(a, b, ws);
PatchList patchList;
try {
patchList = plCache.get(plKey);
patchList = plCache.get(plKey, project);
} catch (PatchListNotAvailableException e) {
throw new SystemException("Cannot create " + plKey);
}

4
gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheModule.java vendored
View File

@@ -19,7 +19,7 @@ import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.Weigher;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.inject.AbstractModule;
import com.google.gerrit.server.config.FactoryModule;
import com.google.inject.Key;
import com.google.inject.Provider;
import com.google.inject.Scopes;
@@ -33,7 +33,7 @@ import java.lang.reflect.Type;
/**
* Miniature DSL to support binding {@link Cache} instances in Guice.
*/
public abstract class CacheModule extends AbstractModule {
public abstract class CacheModule extends FactoryModule {
private static final TypeLiteral<Cache<?, ?>> ANY_CACHE =
new TypeLiteral<Cache<?, ?>>() {};

2
gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java
View File

@@ -55,7 +55,7 @@ public class FileInfoJson {
: ObjectId.fromString(base.getRevision().get());
ObjectId b = ObjectId.fromString(revision.get());
PatchList list = patchListCache.get(
new PatchListKey(change.getProject(), a, b, Whitespace.IGNORE_NONE));
new PatchListKey(a, b, Whitespace.IGNORE_NONE), change.getProject());
Map<String, FileInfo> files = Maps.newTreeMap();
for (PatchListEntry e : list.getPatches()) {

39
gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiffArgs.java Normal file
View File

@@ -0,0 +1,39 @@
// Copyright (C) 2015 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.patch;
import com.google.auto.value.AutoValue;
import com.google.gerrit.reviewdb.client.Project;
import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.lib.ObjectId;
import java.util.List;
@AutoValue
public abstract class IntraLineDiffArgs {
public static IntraLineDiffArgs create(Text aText, Text bText, List<Edit> edits,
Project.NameKey project, ObjectId commit, String path) {
return new AutoValue_IntraLineDiffArgs(aText, bText, edits,
project, commit, path);
}
public abstract Text aText();
public abstract Text bText();
public abstract List<Edit> edits();
public abstract Project.NameKey project();
public abstract ObjectId commit();
public abstract String path();
}

53
gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiffKey.java
View File

@@ -17,16 +17,12 @@ package com.google.gerrit.server.patch;
import static org.eclipse.jgit.lib.ObjectIdSerialization.readNotNull;
import static org.eclipse.jgit.lib.ObjectIdSerialization.writeNotNull;
import com.google.gerrit.reviewdb.client.Project;
import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.lib.ObjectId;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.List;
public class IntraLineDiffKey implements Serializable {
static final long serialVersionUID = 4L;
@@ -35,45 +31,13 @@ public class IntraLineDiffKey implements Serializable {
private transient ObjectId aId;
private transient ObjectId bId;
// Transient data passed through on cache misses to the loader.
private transient Text aText;
private transient Text bText;
private transient List<Edit> edits;
private transient Project.NameKey projectKey;
private transient ObjectId commit;
private transient String path;
public IntraLineDiffKey(ObjectId aId, Text aText, ObjectId bId, Text bText,
List<Edit> edits, Project.NameKey projectKey, ObjectId commit, String path,
public IntraLineDiffKey(ObjectId aId, ObjectId bId,
boolean ignoreWhitespace) {
this.aId = aId;
this.bId = bId;
this.aText = aText;
this.bText = bText;
this.edits = edits;
this.projectKey = projectKey;
this.commit = commit;
this.path = path;
this.ignoreWhitespace = ignoreWhitespace;
}
Text getTextA() {
return aText;
}
Text getTextB() {
return bText;
}
List<Edit> getEdits() {
return edits;
}
public ObjectId getBlobA() {
return aId;
}
@@ -86,18 +50,6 @@ public class IntraLineDiffKey implements Serializable {
return ignoreWhitespace;
}
Project.NameKey getProject() {
return projectKey;
}
ObjectId getCommit() {
return commit;
}
String getPath() {
return path;
}
@Override
public int hashCode() {
int h = 0;
@@ -124,9 +76,6 @@ public class IntraLineDiffKey implements Serializable {
public String toString() {
StringBuilder n = new StringBuilder();
n.append("IntraLineDiffKey[");
if (projectKey != null) {
n.append(projectKey.get()).append(" ");
}
n.append(aId.name());
n.append("..");
n.append(bId.name());

55
gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java
View File

@@ -16,10 +16,10 @@
package com.google.gerrit.server.patch;
import com.google.common.base.Throwables;
import com.google.common.cache.CacheLoader;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.diff.MyersDiff;
@@ -28,7 +28,6 @@ import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@@ -38,9 +37,13 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.regex.Pattern;
class IntraLineLoader extends CacheLoader<IntraLineDiffKey, IntraLineDiff> {
class IntraLineLoader implements Callable<IntraLineDiff> {
static final Logger log = LoggerFactory.getLogger(IntraLineLoader.class);
static interface Factory {
IntraLineLoader create(IntraLineDiffKey key, IntraLineDiffArgs args);
}
private static final Pattern BLANK_LINE_RE = Pattern
.compile("^[ \\t]*(|[{}]|/\\*\\*?|\\*)[ \\t]*$");
@@ -49,32 +52,40 @@ class IntraLineLoader extends CacheLoader<IntraLineDiffKey, IntraLineDiff> {
private final ExecutorService diffExecutor;
private final long timeoutMillis;
private final IntraLineDiffKey key;
private final IntraLineDiffArgs args;
@Inject
@AssistedInject
IntraLineLoader(@DiffExecutor ExecutorService diffExecutor,
@GerritServerConfig Config cfg) {
@GerritServerConfig Config cfg,
@Assisted IntraLineDiffKey key,
@Assisted IntraLineDiffArgs args) {
this.diffExecutor = diffExecutor;
timeoutMillis =
ConfigUtil.getTimeUnit(cfg, "cache", PatchListCacheImpl.INTRA_NAME,
"timeout", TimeUnit.MILLISECONDS.convert(5, TimeUnit.SECONDS),
TimeUnit.MILLISECONDS);
this.key = key;
this.args = args;
}
@Override
public IntraLineDiff load(final IntraLineDiffKey key) throws Exception {
Future<IntraLineDiff> result = diffExecutor.submit(new Callable<IntraLineDiff>() {
@Override
public IntraLineDiff call() throws Exception {
return IntraLineLoader.compute(key);
}
});
public IntraLineDiff call() throws Exception {
Future<IntraLineDiff> result = diffExecutor.submit(
new Callable<IntraLineDiff>() {
@Override
public IntraLineDiff call() throws Exception {
return IntraLineLoader.compute(args.aText(), args.bText(),
args.edits());
}
});
try {
return result.get(timeoutMillis, TimeUnit.MILLISECONDS);
} catch (InterruptedException | TimeoutException e) {
log.warn(timeoutMillis + " ms timeout reached for IntraLineDiff"
+ " in project " + key.getProject().get()
+ " on commit " + key.getCommit().name()
+ " for path " + key.getPath()
+ " in project " + args.project()
+ " on commit " + args.commit().name()
+ " for path " + args.path()
+ " comparing " + key.getBlobA().name()
+ ".." + key.getBlobB().name());
result.cancel(true);
@@ -87,18 +98,16 @@ class IntraLineLoader extends CacheLoader<IntraLineDiffKey, IntraLineDiff> {
}
}
static IntraLineDiff compute(IntraLineDiffKey key) throws Exception {
List<Edit> edits = new ArrayList<>(key.getEdits());
Text aContent = key.getTextA();
Text bContent = key.getTextB();
combineLineEdits(edits, aContent, bContent);
static IntraLineDiff compute(Text aText, Text bText, List<Edit> edits)
throws Exception {
combineLineEdits(edits, aText, bText);
for (int i = 0; i < edits.size(); i++) {
Edit e = edits.get(i);
if (e.getType() == Edit.Type.REPLACE) {
CharText a = new CharText(aContent, e.getBeginA(), e.getEndA());
CharText b = new CharText(bContent, e.getBeginB(), e.getEndB());
CharText a = new CharText(aText, e.getBeginA(), e.getEndA());
CharText b = new CharText(bText, e.getBeginB(), e.getEndB());
CharTextComparator cmp = new CharTextComparator();
List<Edit> wordEdits = MyersDiff.INSTANCE.diff(cmp, a, b);

7
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
View File

@@ -16,13 +16,16 @@ package com.google.gerrit.server.patch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
/** Provides a cached list of {@link PatchListEntry}. */
public interface PatchListCache {
public PatchList get(PatchListKey key) throws PatchListNotAvailableException;
public PatchList get(PatchListKey key, Project.NameKey project)
throws PatchListNotAvailableException;
public PatchList get(Change change, PatchSet patchSet)
throws PatchListNotAvailableException;
public IntraLineDiff getIntraLineDiff(IntraLineDiffKey key);
public IntraLineDiff getIntraLineDiff(IntraLineDiffKey key,
IntraLineDiffArgs args);
}

42
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
View File

@@ -15,7 +15,7 @@
package com.google.gerrit.server.patch;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.Cache;
import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -43,14 +43,14 @@ public class PatchListCacheImpl implements PatchListCache {
return new CacheModule() {
@Override
protected void configure() {
factory(PatchListLoader.Factory.class);
persist(FILE_NAME, PatchListKey.class, PatchList.class)
.maximumWeight(10 << 20)
.loader(PatchListLoader.class)
.weigher(PatchListWeigher.class);
factory(IntraLineLoader.Factory.class);
persist(INTRA_NAME, IntraLineDiffKey.class, IntraLineDiff.class)
.maximumWeight(10 << 20)
.loader(IntraLineLoader.class)
.weigher(IntraLineWeigher.class);
bind(PatchListCacheImpl.class);
@@ -59,17 +59,23 @@ public class PatchListCacheImpl implements PatchListCache {
};
}
private final LoadingCache<PatchListKey, PatchList> fileCache;
private final LoadingCache<IntraLineDiffKey, IntraLineDiff> intraCache;
private final Cache<PatchListKey, PatchList> fileCache;
private final Cache<IntraLineDiffKey, IntraLineDiff> intraCache;
private final PatchListLoader.Factory fileLoaderFactory;
private final IntraLineLoader.Factory intraLoaderFactory;
private final boolean computeIntraline;
@Inject
PatchListCacheImpl(
@Named(FILE_NAME) LoadingCache<PatchListKey, PatchList> fileCache,
@Named(INTRA_NAME) LoadingCache<IntraLineDiffKey, IntraLineDiff> intraCache,
@Named(FILE_NAME) Cache<PatchListKey, PatchList> fileCache,
@Named(INTRA_NAME) Cache<IntraLineDiffKey, IntraLineDiff> intraCache,
PatchListLoader.Factory fileLoaderFactory,
IntraLineLoader.Factory intraLoaderFactory,
@GerritServerConfig Config cfg) {
this.fileCache = fileCache;
this.intraCache = intraCache;
this.fileLoaderFactory = fileLoaderFactory;
this.intraLoaderFactory = intraLoaderFactory;
this.computeIntraline =
cfg.getBoolean("cache", INTRA_NAME, "enabled",
@@ -77,9 +83,10 @@ public class PatchListCacheImpl implements PatchListCache {
}
@Override
public PatchList get(PatchListKey key) throws PatchListNotAvailableException {
public PatchList get(PatchListKey key, Project.NameKey project)
throws PatchListNotAvailableException {
try {
return fileCache.get(key);
return fileCache.get(key, fileLoaderFactory.create(key, project));
} catch (ExecutionException | LargeObjectException e) {
PatchListLoader.log.warn("Error computing " + key, e);
throw new PatchListNotAvailableException(e.getCause());
@@ -87,24 +94,25 @@ public class PatchListCacheImpl implements PatchListCache {
}
@Override
public PatchList get(final Change change, final PatchSet patchSet)
public PatchList get(Change change, PatchSet patchSet)
throws PatchListNotAvailableException {
final Project.NameKey projectKey = change.getProject();
final ObjectId a = null;
Project.NameKey project = change.getProject();
ObjectId a = null;
if (patchSet.getRevision() == null) {
throw new PatchListNotAvailableException(
"revision is null for " + patchSet.getId());
}
final ObjectId b = ObjectId.fromString(patchSet.getRevision().get());
final Whitespace ws = Whitespace.IGNORE_NONE;
return get(new PatchListKey(projectKey, a, b, ws));
ObjectId b = ObjectId.fromString(patchSet.getRevision().get());
Whitespace ws = Whitespace.IGNORE_NONE;
return get(new PatchListKey(a, b, ws), project);
}
@Override
public IntraLineDiff getIntraLineDiff(IntraLineDiffKey key) {
public IntraLineDiff getIntraLineDiff(IntraLineDiffKey key,
IntraLineDiffArgs args) {
if (computeIntraline) {
try {
return intraCache.get(key);
return intraCache.get(key, intraLoaderFactory.create(key, args));
} catch (ExecutionException | LargeObjectException e) {
IntraLineLoader.log.warn("Error computing " + key, e);
return new IntraLineDiff(IntraLineDiff.Status.ERROR);

11
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
View File

@@ -23,7 +23,6 @@ import static org.eclipse.jgit.lib.ObjectIdSerialization.writeNotNull;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace;
import com.google.gerrit.reviewdb.client.Project;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
@@ -40,11 +39,7 @@ public class PatchListKey implements Serializable {
private transient ObjectId newId;
private transient Whitespace whitespace;
transient Project.NameKey projectKey; // not required to form the key
public PatchListKey(final Project.NameKey pk, final AnyObjectId a,
final AnyObjectId b, final Whitespace ws) {
projectKey = pk;
public PatchListKey(AnyObjectId a, AnyObjectId b, Whitespace ws) {
oldId = a != null ? a.copy() : null;
newId = b.copy();
whitespace = ws;
@@ -94,10 +89,6 @@ public class PatchListKey implements Serializable {
public String toString() {
StringBuilder n = new StringBuilder();
n.append("PatchListKey[");
if (projectKey != null) {
n.append(projectKey.get());
n.append(" ");
}
n.append(oldId != null ? oldId.name() : "BASE");
n.append("..");
n.append(newId.name());

66
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
View File

@@ -17,17 +17,17 @@ package com.google.gerrit.server.patch;
import com.google.common.base.Function;
import com.google.common.base.Throwables;
import com.google.common.cache.CacheLoader;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.Iterables;
import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.diff.DiffFormatter;
@@ -79,25 +79,34 @@ import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
public class PatchListLoader extends CacheLoader<PatchListKey, PatchList> {
public class PatchListLoader implements Callable<PatchList> {
static final Logger log = LoggerFactory.getLogger(PatchListLoader.class);
public interface Factory {
PatchListLoader create(PatchListKey key, Project.NameKey project);
}
private final GitRepositoryManager repoManager;
private final PatchListCache patchListCache;
private final ThreeWayMergeStrategy mergeStrategy;
private final ExecutorService diffExecutor;
private final PatchListKey key;
private final Project.NameKey project;
private final long timeoutMillis;
@Inject
@AssistedInject
PatchListLoader(GitRepositoryManager mgr,
PatchListCache plc,
@GerritServerConfig Config cfg,
@DiffExecutor ExecutorService de) {
@DiffExecutor ExecutorService de,
@Assisted PatchListKey k,
@Assisted Project.NameKey p) {
repoManager = mgr;
patchListCache = plc;
mergeStrategy = MergeUtil.getMergeStrategy(cfg);
diffExecutor = de;
key = k;
project = p;
timeoutMillis =
ConfigUtil.getTimeUnit(cfg, "cache", PatchListCacheImpl.FILE_NAME,
"timeout", TimeUnit.MILLISECONDS.convert(5, TimeUnit.SECONDS),
@@ -105,9 +114,9 @@ public class PatchListLoader extends CacheLoader<PatchListKey, PatchList> {
}
@Override
public PatchList load(final PatchListKey key) throws IOException,
public PatchList call() throws IOException,
PatchListNotAvailableException {
try (Repository repo = repoManager.openRepository(key.projectKey)) {
try (Repository repo = repoManager.openRepository(project)) {
return readPatchList(key, repo);
}
}
@@ -159,25 +168,24 @@ public class PatchListLoader extends CacheLoader<PatchListKey, PatchList> {
df.setDetectRenames(true);
List<DiffEntry> diffEntries = df.scan(aTree, bTree);
Set<String> paths = key.getOldId() != null
? FluentIterable.from(
Iterables.concat(
patchListCache.get(
new PatchListKey(key.projectKey, null,
key.getNewId(), key.getWhitespace()))
.getPatches(),
patchListCache.get(
new PatchListKey(key.projectKey, null,
key.getOldId(), key.getWhitespace()))
.getPatches()))
.transform(new Function<PatchListEntry, String>() {
@Override
public String apply(PatchListEntry entry) {
return entry.getNewName();
}
})
.toSet()
: null;
Set<String> paths = null;
if (key.getOldId() != null) {
PatchListKey newKey =
new PatchListKey(null, key.getNewId(), key.getWhitespace());
PatchListKey oldKey =
new PatchListKey(null, key.getOldId(), key.getWhitespace());
paths = FluentIterable
.from(patchListCache.get(newKey, project).getPatches())
.append(patchListCache.get(oldKey, project).getPatches())
.transform(new Function<PatchListEntry, String>() {
@Override
public String apply(PatchListEntry entry) {
return entry.getNewName();
}
})
.toSet();
}
int cnt = diffEntries.size();
List<PatchListEntry> entries = new ArrayList<>();
entries.add(newCommitMessage(cmp, reader,
@@ -210,7 +218,7 @@ public class PatchListLoader extends CacheLoader<PatchListKey, PatchList> {
return result.get(timeoutMillis, TimeUnit.MILLISECONDS);
} catch (InterruptedException | TimeoutException e) {
log.warn(timeoutMillis + " ms timeout reached for Diff loader"
+ " in project " + key.projectKey.get()
+ " in project " + project
+ " on commit " + key.getNewId().name()
+ " on path " + diffEntry.getNewPath()
+ " comparing " + diffEntry.getOldId().name()

9
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
View File

@@ -142,9 +142,12 @@ class PatchScriptBuilder {
intralineDifferenceIsPossible = false;
} else if (diffPrefs.isIntralineDifference()) {
IntraLineDiff d =
patchListCache.getIntraLineDiff(new IntraLineDiffKey(a.id, a.src,
b.id, b.src, edits, projectKey, bId, b.path,
diffPrefs.getIgnoreWhitespace() != Whitespace.IGNORE_NONE));
patchListCache.getIntraLineDiff(
new IntraLineDiffKey(
a.id, b.id,
diffPrefs.getIgnoreWhitespace() != Whitespace.IGNORE_NONE),
IntraLineDiffArgs.create(
a.src, b.src, edits, projectKey, bId, b.path));
if (d != null) {
switch (d.getStatus()) {
case EDIT_LIST:

16
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
View File

@@ -93,7 +93,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
private boolean loadComments = true;
private Change change;
private Project.NameKey projectKey;
private Project.NameKey project;
private ChangeControl control;
private ObjectId aId;
private ObjectId bId;
@@ -145,7 +145,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
validatePatchSetId(psb);
change = control.getChange();
projectKey = change.getProject();
project = change.getProject();
aId = psa != null ? toObjectId(db, psa) : null;
bId = toObjectId(db, psb);
@@ -155,7 +155,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
throw new NoSuchChangeException(changeId);
}
try (Repository git = repoManager.openRepository(projectKey)) {
try (Repository git = repoManager.openRepository(project)) {
try {
final PatchList list = listFor(keyFor(diffPrefs.getIgnoreWhitespace()));
final PatchScriptBuilder b = newBuilder(list, git);
@@ -175,27 +175,27 @@ public class PatchScriptFactory implements Callable<PatchScript> {
throw new LargeObjectException("File content is too large", err);
}
} catch (RepositoryNotFoundException e) {
log.error("Repository " + projectKey + " not found", e);
log.error("Repository " + project + " not found", e);
throw new NoSuchChangeException(changeId, e);
} catch (IOException e) {
log.error("Cannot open repository " + projectKey, e);
log.error("Cannot open repository " + project, e);
throw new NoSuchChangeException(changeId, e);
}
}
private PatchListKey keyFor(final Whitespace whitespace) {
return new PatchListKey(projectKey, aId, bId, whitespace);
return new PatchListKey(aId, bId, whitespace);
}
private PatchList listFor(final PatchListKey key)
throws PatchListNotAvailableException {
return patchListCache.get(key);
return patchListCache.get(key, project);
}
private PatchScriptBuilder newBuilder(final PatchList list, Repository git) {
final AccountDiffPreference dp = new AccountDiffPreference(diffPrefs);
final PatchScriptBuilder b = builderFactory.get();
b.setRepository(git, projectKey);
b.setRepository(git, project);
b.setChange(change);
b.setDiffPrefs(dp);
b.setTrees(list.isAgainstParent(), list.getOldId(), list.getNewId());