ChangeData: Cache PatchLists locally
If a PatchList is not available e.g. due to LargeObjectException, it's likely to be unavailable for the lifetime of a single request, which is the expected lifetime of a ChangeData. In the case of indexing a change, the various fields that depend on the PatchList know how to handle missing data, which is just to not emit the field. However, attempting to load PatchList multiple times if it's almost certainly going to fail multiple times is wasted effort. Cache the failure instead. Change-Id: I714e6c704b470eedd6c55124bbd63a1afa156a7e
This commit is contained in:
@@ -332,6 +332,7 @@ public class ChangeData {
|
||||
private ListMultimap<PatchSet.Id, PatchSetApproval> allApprovals;
|
||||
private List<PatchSetApproval> currentApprovals;
|
||||
private Map<Integer, List<String>> files = new HashMap<>();
|
||||
private Map<Integer, Optional<PatchList>> patchLists = new HashMap<>();
|
||||
private Collection<PatchLineComment> publishedComments;
|
||||
private CurrentUser visibleTo;
|
||||
private ChangeControl changeControl;
|
||||
@@ -565,23 +566,23 @@ public class ChangeData {
|
||||
}
|
||||
|
||||
public List<String> filePaths(PatchSet ps) throws OrmException {
|
||||
if (!files.containsKey(ps.getPatchSetId())) {
|
||||
Integer psId = ps.getPatchSetId();
|
||||
List<String> r = files.get(psId);
|
||||
if (r == null) {
|
||||
Change c = change();
|
||||
if (c == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
PatchList p;
|
||||
try {
|
||||
p = patchListCache.get(c, ps);
|
||||
} catch (PatchListNotAvailableException e) {
|
||||
Optional<PatchList> p = getPatchList(c, ps);
|
||||
if (!p.isPresent()) {
|
||||
List<String> emptyFileList = Collections.emptyList();
|
||||
files.put(ps.getPatchSetId(), emptyFileList);
|
||||
return emptyFileList;
|
||||
}
|
||||
|
||||
List<String> r = new ArrayList<>(p.getPatches().size());
|
||||
for (PatchListEntry e : p.getPatches()) {
|
||||
r = new ArrayList<>(p.get().getPatches().size());
|
||||
for (PatchListEntry e : p.get().getPatches()) {
|
||||
if (Patch.COMMIT_MSG.equals(e.getNewName())) {
|
||||
continue;
|
||||
}
|
||||
@@ -601,9 +602,24 @@ public class ChangeData {
|
||||
}
|
||||
}
|
||||
Collections.sort(r);
|
||||
files.put(ps.getPatchSetId(), Collections.unmodifiableList(r));
|
||||
r = Collections.unmodifiableList(r);
|
||||
files.put(psId, r);
|
||||
}
|
||||
return files.get(ps.getPatchSetId());
|
||||
return r;
|
||||
}
|
||||
|
||||
private Optional<PatchList> getPatchList(Change c, PatchSet ps) {
|
||||
Integer psId = ps.getId().get();
|
||||
Optional<PatchList> r = patchLists.get(psId);
|
||||
if (r == null) {
|
||||
try {
|
||||
r = Optional.of(patchListCache.get(c, ps));
|
||||
} catch (PatchListNotAvailableException e) {
|
||||
r = Optional.absent();
|
||||
}
|
||||
patchLists.put(psId, r);
|
||||
}
|
||||
return r;
|
||||
}
|
||||
|
||||
private Optional<ChangedLines> computeChangedLines() throws OrmException {
|
||||
@@ -615,13 +631,12 @@ public class ChangeData {
|
||||
if (ps == null) {
|
||||
return Optional.absent();
|
||||
}
|
||||
try {
|
||||
PatchList p = patchListCache.get(c, ps);
|
||||
return Optional.of(
|
||||
new ChangedLines(p.getInsertions(), p.getDeletions()));
|
||||
} catch (PatchListNotAvailableException e) {
|
||||
Optional<PatchList> p = getPatchList(c, ps);
|
||||
if (!p.isPresent()) {
|
||||
return Optional.absent();
|
||||
}
|
||||
return Optional.of(
|
||||
new ChangedLines(p.get().getInsertions(), p.get().getDeletions()));
|
||||
}
|
||||
|
||||
public Optional<ChangedLines> changedLines() throws OrmException {
|
||||
|
Reference in New Issue
Block a user