diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 0f186b9df1..3d391877a8 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -611,6 +611,7 @@ Default is 1024 for most caches, except: * `"adv_bases"`: default is `4096` * `"diff"`: default is `10m` (10 MiB of memory) * `"diff_intraline"`: default is `10m` (10 MiB of memory) +* `"diff_file_list"`: default is `10m` (10 MiB of memory) * `"plugin_resources"`: default is 2m (2 MiB of memory) + @@ -626,7 +627,10 @@ accessed order until the cache fits within this limit. Caches may grow larger than this during the day, as the size check is only performed once every 24 hours. + -Default is 128 MiB per cache. +Default is 128 MiB per cache, except: ++ +* `"diff_file_list"`: default is `1g` (1 GiB of disk space) + + If 0, disk storage for the cache is disabled. @@ -698,6 +702,16 @@ estimate in bytes of memory used. Administrators should try to target cache.diff.memoryLimit to fit all files users will view in a 1 or 2 day span. +cache `"diff_file_list"`:: ++ +Each item caches list of file paths which are different between two +commits. Gerrit uses this cache to accelerate computing of the list +of paths of changed files. ++ +Ideally, disk limit of this cache is large enough to cover all changes. +This should significantly speed up change reindexing, especially +full offline reindexing. + cache `"git_tags"`:: + If branch or reference level READ access controls are used, this diff --git a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java index 8141dfb149..f7381a39c8 100644 --- a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java +++ b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java @@ -178,7 +178,7 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener { public LoadingCache build( CacheBinding def, CacheLoader loader) { - long limit = config.getLong("cache", def.name(), "diskLimit", 128 << 20); + long limit = config.getLong("cache", def.name(), "diskLimit", def.diskLimit()); if (cacheDir == null || limit <= 0) { return defaultFactory.build(def, loader); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheBinding.java b/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheBinding.java index 70628718de..343827cb83 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheBinding.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheBinding.java @@ -26,6 +26,9 @@ public interface CacheBinding { /** Set the total size of the cache. */ CacheBinding maximumWeight(long weight); + /** Set the total on-disk limit of the cache */ + CacheBinding diskLimit(long limit); + /** Set the time an element lives before being expired. */ CacheBinding expireAfterWrite(long duration, TimeUnit durationUnits); @@ -39,6 +42,7 @@ public interface CacheBinding { TypeLiteral keyType(); TypeLiteral valueType(); long maximumWeight(); + long diskLimit(); @Nullable Long expireAfterWrite(TimeUnit unit); @Nullable Weigher weigher(); @Nullable CacheLoader loader(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheProvider.java index 6d9ae0f34b..c73760c259 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/cache/CacheProvider.java @@ -38,6 +38,7 @@ class CacheProvider private final TypeLiteral valType; private boolean persist; private long maximumWeight; + private long diskLimit; private Long expireAfterWrite; private Provider> loader; private Provider> weigher; @@ -85,6 +86,15 @@ class CacheProvider return this; } + @Override + public CacheBinding diskLimit(long limit) { + Preconditions.checkState(!frozen, "binding frozen, cannot be modified"); + Preconditions.checkState(persist, + "diskLimit supported for persistent caches only"); + diskLimit = limit; + return this; + } + @Override public CacheBinding expireAfterWrite(long duration, TimeUnit unit) { Preconditions.checkState(!frozen, "binding frozen, cannot be modified"); @@ -129,6 +139,14 @@ class CacheProvider return maximumWeight; } + @Override + public long diskLimit() { + if (diskLimit > 0) { + return diskLimit; + } + return 128 << 20; + } + @Override @Nullable public Long expireAfterWrite(TimeUnit unit) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileList.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileList.java new file mode 100644 index 0000000000..7715f7d789 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileList.java @@ -0,0 +1,65 @@ +// Copyright (C) 2016 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 static com.google.gerrit.server.ioutil.BasicSerialization.readBytes; +import static com.google.gerrit.server.ioutil.BasicSerialization.readString; +import static com.google.gerrit.server.ioutil.BasicSerialization.writeBytes; +import static com.google.gerrit.server.ioutil.BasicSerialization.writeString; + +import com.google.common.base.Joiner; +import com.google.common.base.Splitter; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.zip.DeflaterOutputStream; +import java.util.zip.InflaterInputStream; + +public class FileList implements Serializable { + private static final long serialVersionUID = PatchListKey.serialVersionUID; + + private transient String[] paths; + + public FileList(String[] paths) { + this.paths = paths; + } + + public List getPaths() { + return Collections.unmodifiableList(Arrays.asList(paths)); + } + + private void writeObject(ObjectOutputStream output) throws IOException { + ByteArrayOutputStream buf = new ByteArrayOutputStream(); + try (DeflaterOutputStream out = new DeflaterOutputStream(buf)) { + writeString(out, Joiner.on('\n').join(paths)); + } + writeBytes(output, buf.toByteArray()); + } + + private void readObject(ObjectInputStream input) throws IOException { + ByteArrayInputStream buf = new ByteArrayInputStream(readBytes(input)); + try (InflaterInputStream in = new InflaterInputStream(buf)) { + List l = Splitter.on('\n').splitToList(readString(in)); + paths = l.toArray(new String[l.size()]); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileListLoader.java new file mode 100644 index 0000000000..f73c139187 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileListLoader.java @@ -0,0 +1,80 @@ +// Copyright (C) 2016 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.gerrit.reviewdb.client.Patch; +import com.google.gerrit.reviewdb.client.Project; +import com.google.inject.assistedinject.Assisted; +import com.google.inject.assistedinject.AssistedInject; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Callable; + +public class FileListLoader implements Callable { + static final Logger log = LoggerFactory.getLogger(FileListLoader.class); + + public interface Factory { + FileListLoader create(PatchListKey key, Project.NameKey project); + } + + private final PatchListCache patchListCache; + private final PatchListKey key; + private final Project.NameKey project; + + @AssistedInject + FileListLoader(PatchListCache plc, + @Assisted PatchListKey k, + @Assisted Project.NameKey p) { + patchListCache = plc; + key = k; + project = p; + } + + @Override + public FileList call() throws Exception { + PatchList patchList = patchListCache.get(key, project); + return toFileList(patchList); + } + + static FileList toFileList(PatchList patchList) { + List r = new ArrayList<>(patchList.getPatches().size()); + for (PatchListEntry e : patchList.getPatches()) { + if (Patch.isMagic(e.getNewName())) { + continue; + } + switch (e.getChangeType()) { + case ADDED: + case MODIFIED: + case DELETED: + case COPIED: + case REWRITE: + r.add(e.getNewName()); + break; + + case RENAMED: + r.add(e.getOldName()); + r.add(e.getNewName()); + break; + } + } + Collections.sort(r); + return new FileList(r.toArray(new String[r.size()])); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileListWeigher.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileListWeigher.java new file mode 100644 index 0000000000..7e884f7702 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/FileListWeigher.java @@ -0,0 +1,35 @@ +// Copyright (C) 2016 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.common.cache.Weigher; + +/** Computes memory usage for FileList in bytes of memory used */ +public class FileListWeigher implements Weigher { + + @Override + public int weigh(PatchListKey key, FileList value) { + int size = 16 + 4 * 8 + 2 * 36; // Size of PatchListKey, 64 bit JVM + + // Size of the list of paths ... + for (String p : value.getPaths()) { + size += p.length(); + } + // ... plus new-line separators between paths + size += value.getPaths().size() - 1; + + return size; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java index 8a2403f40f..7b413f663f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java @@ -33,4 +33,7 @@ public interface PatchListCache { IntraLineDiff getIntraLineDiff(IntraLineDiffKey key, IntraLineDiffArgs args); + + FileList getFileList(Change change, PatchSet patchSet) + throws PatchListNotAvailableException; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java index abafad798a..3e96ab513d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.patch; +import static com.google.gerrit.server.patch.FileListLoader.toFileList; import com.google.common.cache.Cache; import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; @@ -39,6 +40,7 @@ import java.util.concurrent.ExecutionException; public class PatchListCacheImpl implements PatchListCache { static final String FILE_NAME = "diff"; static final String INTRA_NAME = "diff_intraline"; + static final String FILE_LIST = "diff_file_list"; public static Module module() { return new CacheModule() { @@ -54,6 +56,12 @@ public class PatchListCacheImpl implements PatchListCache { .maximumWeight(10 << 20) .weigher(IntraLineWeigher.class); + factory(FileListLoader.Factory.class); + persist(FILE_LIST, PatchListKey.class, FileList.class) + .maximumWeight(10 << 20) + .weigher(FileListWeigher.class) + .diskLimit(1 << 30); + bind(PatchListCacheImpl.class); bind(PatchListCache.class).to(PatchListCacheImpl.class); } @@ -62,21 +70,27 @@ public class PatchListCacheImpl implements PatchListCache { private final Cache fileCache; private final Cache intraCache; + private final Cache fileListCache; private final PatchListLoader.Factory fileLoaderFactory; private final IntraLineLoader.Factory intraLoaderFactory; + private final FileListLoader.Factory fileListLoaderFactory; private final boolean computeIntraline; @Inject PatchListCacheImpl( @Named(FILE_NAME) Cache fileCache, @Named(INTRA_NAME) Cache intraCache, + @Named(FILE_LIST) Cache fileListCache, PatchListLoader.Factory fileLoaderFactory, IntraLineLoader.Factory intraLoaderFactory, + FileListLoader.Factory fileListLoaderFactory, @GerritServerConfig Config cfg) { this.fileCache = fileCache; this.intraCache = intraCache; + this.fileListCache = fileListCache; this.fileLoaderFactory = fileLoaderFactory; this.intraLoaderFactory = intraLoaderFactory; + this.fileListLoaderFactory = fileListLoaderFactory; this.computeIntraline = cfg.getBoolean("cache", INTRA_NAME, "enabled", @@ -87,7 +101,9 @@ public class PatchListCacheImpl implements PatchListCache { public PatchList get(PatchListKey key, Project.NameKey project) throws PatchListNotAvailableException { try { - return fileCache.get(key, fileLoaderFactory.create(key, project)); + PatchList pl = fileCache.get(key, fileLoaderFactory.create(key, project)); + fileListCache.put(key, toFileList(pl)); + return pl; } catch (ExecutionException e) { PatchListLoader.log.warn("Error computing " + key, e); throw new PatchListNotAvailableException(e); @@ -140,4 +156,30 @@ public class PatchListCacheImpl implements PatchListCache { } return new IntraLineDiff(IntraLineDiff.Status.DISABLED); } -} + + @Override + public FileList getFileList(Change change, PatchSet patchSet) + throws PatchListNotAvailableException { + Project.NameKey project = change.getProject(); + ObjectId b = ObjectId.fromString(patchSet.getRevision().get()); + Whitespace ws = Whitespace.IGNORE_NONE; + return getFileList(PatchListKey.againstDefaultBase(b, ws), project); + } + + private FileList getFileList(PatchListKey key, Project.NameKey project) + throws PatchListNotAvailableException { + try { + return fileListCache.get(key, + fileListLoaderFactory.create(key, project)); + } catch (ExecutionException e) { + PatchListLoader.log.warn("Error computing " + key, e); + throw new PatchListNotAvailableException(e); + } catch (UncheckedExecutionException e) { + if (e.getCause() instanceof LargeObjectException) { + PatchListLoader.log.warn("Error computing " + key, e); + throw new PatchListNotAvailableException(e); + } + throw e; + } + } +} \ No newline at end of file diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index b3c81f42c1..1b74f28e19 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -38,7 +38,6 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.Comment; -import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; @@ -59,9 +58,9 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.NotesMigration; +import com.google.gerrit.server.patch.FileList; import com.google.gerrit.server.patch.PatchList; import com.google.gerrit.server.patch.PatchListCache; -import com.google.gerrit.server.patch.PatchListEntry; import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; @@ -337,6 +336,7 @@ public class ChangeData { private List currentApprovals; private Map> files; private Map> patchLists; + private Map> fileLists; private Collection publishedComments; private CurrentUser visibleTo; private ChangeControl changeControl; @@ -589,7 +589,7 @@ public class ChangeData { return null; } - Optional p = getPatchList(c, ps); + Optional p = getFileList(c, ps); if (!p.isPresent()) { List emptyFileList = Collections.emptyList(); if (lazyLoad) { @@ -598,28 +598,7 @@ public class ChangeData { return emptyFileList; } - r = new ArrayList<>(p.get().getPatches().size()); - for (PatchListEntry e : p.get().getPatches()) { - if (Patch.isMagic(e.getNewName())) { - continue; - } - switch (e.getChangeType()) { - case ADDED: - case MODIFIED: - case DELETED: - case COPIED: - case REWRITE: - r.add(e.getNewName()); - break; - - case RENAMED: - r.add(e.getOldName()); - r.add(e.getNewName()); - break; - } - } - Collections.sort(r); - r = Collections.unmodifiableList(r); + r = p.get().getPaths(); files.put(psId, r); } return r; @@ -645,6 +624,26 @@ public class ChangeData { return r; } + private Optional getFileList(Change c, PatchSet ps) { + Integer psId = ps.getId().get(); + if (fileLists == null) { + fileLists = new HashMap<>(); + } + Optional r = fileLists.get(psId); + if (r == null) { + if (!lazyLoad) { + return Optional.absent(); + } + try { + r = Optional.of(patchListCache.getFileList(c, ps)); + } catch (PatchListNotAvailableException e) { + r = Optional.absent(); + } + fileLists.put(psId, r); + } + return r; + } + private Optional computeChangedLines() throws OrmException { Change c = change(); if (c == null) {