From 25e8ac86d85f1508fe1b242ab72d235057cd8c49 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Wed, 23 Nov 2016 21:54:22 +0300 Subject: [PATCH] [vcs-log] index commits one by one During indexing when only a part of the repository is being processed, commits were loaded in batches by 1000, then each batch was analysed and stored in the index. After that commits themselves were (and still are) thrown away. Since it turned out commits can be quite big, it is unwise to collect them. So now commits are processed immediately as they are loaded. IDEA-164274 --- .../src/com/intellij/vcsUtil/VcsFileUtil.java | 27 ++++++-- .../com/intellij/vcs/log/VcsLogProvider.java | 21 +++++- .../vcs/log/data/CommitDetailsGetter.java | 5 +- .../log/data/index/VcsLogPersistentIndex.java | 3 +- .../vcs/log/impl/TestVcsLogProvider.java | 13 ++-- .../src/git4idea/history/GitHistoryUtils.java | 67 ++++++++++--------- .../src/git4idea/log/GitLogProvider.java | 43 ++++++------ .../src/git4idea/reset/GitUncommitAction.java | 4 +- .../git4idea/history/GitHistoryUtilsTest.java | 4 +- .../git4idea/log/GitLogProviderTest.java | 8 +-- .../org/zmlx/hg4idea/log/HgLogProvider.java | 21 ++++-- 11 files changed, 134 insertions(+), 82 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/vcsUtil/VcsFileUtil.java b/platform/vcs-impl/src/com/intellij/vcsUtil/VcsFileUtil.java index f0179dca135c..98a3d84eb6c2 100644 --- a/platform/vcs-impl/src/com/intellij/vcsUtil/VcsFileUtil.java +++ b/platform/vcs-impl/src/com/intellij/vcsUtil/VcsFileUtil.java @@ -26,6 +26,7 @@ import com.intellij.openapi.vcs.VcsException; import com.intellij.openapi.vcs.changes.VcsDirtyScopeManager; import com.intellij.openapi.vfs.VfsUtil; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.util.ThrowableConsumer; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -62,7 +63,7 @@ public class VcsFileUtil { } /** - * Execute function for each chunk of arguments. Check for being cancelled in process. + * Execute function for each chunk of arguments and collect the result. Check for being cancelled in process. * * @param arguments the arguments to chunk * @param groupSize size of argument groups that should be put in the same chunk (like a name and a value) @@ -77,16 +78,34 @@ public class VcsFileUtil { @NotNull ThrowableNotNullFunction, List, VcsException> processor) throws VcsException { List result = ContainerUtil.newArrayList(); + + foreachChunk(arguments, groupSize, chunk -> { + result.addAll(processor.fun(chunk)); + }); + + return result; + } + + /** + * Execute function for each chunk of arguments. Check for being cancelled in process. + * + * @param arguments the arguments to chunk + * @param groupSize size of argument groups that should be put in the same chunk (like a name and a value) + * @param consumer consumer to feed each chunk + * @throws VcsException + */ + public static void foreachChunk(@NotNull List arguments, + int groupSize, + @NotNull ThrowableConsumer, VcsException> consumer) + throws VcsException { List> chunks = chunkArguments(arguments, groupSize); for (List chunk : chunks) { ProgressIndicator indicator = ProgressManager.getInstance().getProgressIndicator(); if (indicator != null) indicator.checkCanceled(); - result.addAll(processor.fun(chunk)); + consumer.consume(chunk); } - - return result; } /** diff --git a/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLogProvider.java b/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLogProvider.java index 5c9d2bcd3c2a..d067bab9d016 100644 --- a/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLogProvider.java +++ b/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLogProvider.java @@ -5,6 +5,7 @@ import com.intellij.openapi.vcs.VcsException; import com.intellij.openapi.vcs.VcsKey; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.Consumer; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.messages.MessageBus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -47,6 +48,14 @@ public interface VcsLogProvider { */ void readAllFullDetails(@NotNull VirtualFile root, @NotNull Consumer commitConsumer) throws VcsException; + /** + * Reads full details for specified commits in the repository. + *

+ * Reports commits to the consumer to avoid creation & even temporary storage of a too large commits collection. + */ + void readFullDetails(@NotNull VirtualFile root, @NotNull List hashes, @NotNull Consumer commitConsumer) + throws VcsException; + /** * Reads those details of the given commits, which are necessary to be shown in the log table. */ @@ -55,9 +64,19 @@ public interface VcsLogProvider { /** * Read full details of the given commits from the VCS. + *

+ * Replaced with readFullDetails(VirtualFile root, List, Consumer) method. + *

+ * To be removed after 2017.1 release. */ @NotNull - List readFullDetails(@NotNull VirtualFile root, @NotNull List hashes) throws VcsException; + @Deprecated + default List readFullDetails(@NotNull VirtualFile root, @NotNull List hashes) + throws VcsException { + List result = ContainerUtil.newArrayList(); + readFullDetails(root, hashes, result::add); + return result; + } /** *

Returns the VCS which is supported by this provider.

diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CommitDetailsGetter.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CommitDetailsGetter.java index 739de9449db2..5f3df9fe1cb1 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CommitDetailsGetter.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CommitDetailsGetter.java @@ -3,6 +3,7 @@ package com.intellij.vcs.log.data; import com.intellij.openapi.Disposable; import com.intellij.openapi.vcs.VcsException; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.util.containers.ContainerUtil; import com.intellij.vcs.log.VcsFullCommitDetails; import com.intellij.vcs.log.VcsLogProvider; import org.jetbrains.annotations.NotNull; @@ -32,6 +33,8 @@ public class CommitDetailsGetter extends AbstractDataGetter readDetails(@NotNull VcsLogProvider logProvider, @NotNull VirtualFile root, @NotNull List hashes) throws VcsException { - return logProvider.readFullDetails(root, hashes); + List result = ContainerUtil.newArrayList(); + logProvider.readFullDetails(root, hashes, result::add); + return result; } } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java index 6cb776fd59fa..7bb742eb80a9 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java @@ -533,7 +533,8 @@ public class VcsLogPersistentIndex implements VcsLogIndex, Disposable { private boolean indexOneByOne(@NotNull VirtualFile root, @NotNull TIntHashSet commits) { VcsLogProvider provider = myProviders.get(root); try { - storeDetails(provider.readFullDetails(root, TroveUtil.map(commits, value -> myHashMap.getCommitId(value).getHash().asString()))); + List hashes = TroveUtil.map(commits, value -> myHashMap.getCommitId(value).getHash().asString()); + provider.readFullDetails(root, hashes, details -> storeDetails(Collections.singletonList(details))); } catch (VcsException e) { LOG.error(e); diff --git a/platform/vcs-log/impl/test/com/intellij/vcs/log/impl/TestVcsLogProvider.java b/platform/vcs-log/impl/test/com/intellij/vcs/log/impl/TestVcsLogProvider.java index 0ef08b7b0844..7f9b4e69fb6a 100644 --- a/platform/vcs-log/impl/test/com/intellij/vcs/log/impl/TestVcsLogProvider.java +++ b/platform/vcs-log/impl/test/com/intellij/vcs/log/impl/TestVcsLogProvider.java @@ -126,7 +126,14 @@ public class TestVcsLogProvider implements VcsLogProvider { @Override public void readAllFullDetails(@NotNull VirtualFile root, @NotNull Consumer commitConsumer) throws VcsException { + throw new UnsupportedOperationException(); + } + @Override + public void readFullDetails(@NotNull VirtualFile root, + @Nullable List hashes, + @NotNull Consumer commitConsumer) throws VcsException { + throw new UnsupportedOperationException(); } private void assertRoot(@NotNull VirtualFile root) { @@ -140,12 +147,6 @@ public class TestVcsLogProvider implements VcsLogProvider { throw new UnsupportedOperationException(); } - @NotNull - @Override - public List readFullDetails(@NotNull VirtualFile root, @NotNull List hashes) throws VcsException { - throw new UnsupportedOperationException(); - } - @NotNull @Override public VcsKey getSupportedVcs() { diff --git a/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java b/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java index d1441889d91a..b73587d70f5f 100644 --- a/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java +++ b/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java @@ -635,25 +635,6 @@ public class GitHistoryUtils { } } - /* - Unlike loadDetails, which accepts list of hashes in parameters, loads details for all commits in the repository. - To optimize memory consumption, git log command output is parsed on-the-fly and resulting commits are immediately fed to the consumer - and not stored in memory. - */ - public static void loadAllDetails(@NotNull Project project, - @NotNull VirtualFile root, - @NotNull Consumer commitConsumer) throws VcsException { - final VcsLogObjectsFactory factory = getObjectsFactoryWithDisposeCheck(project); - if (factory == null) { - return; - } - - GitLineHandler h = new GitLineHandler(project, root, GitCommand.LOG); - GitLogParser parser = createParserForDetails(h, project, false, true, ArrayUtil.toStringArray(LOG_ALL)); - - processHandlerOutputByLine(h, parser, record -> commitConsumer.consume(createCommit(project, root, record, factory))); - } - public static void readCommits(@NotNull Project project, @NotNull VirtualFile root, @NotNull List parameters, @@ -877,7 +858,7 @@ public class GitHistoryUtils { } final Set refs = new OpenTHashSet<>(GitLogProvider.DONT_CONSIDER_SHA); final List commits = - loadDetails(project, root, true, false, record -> { + collectDetails(project, root, true, false, record -> { GitCommit commit = createCommit(project, root, record, factory); Collection refsInRecord = parseRefs(record.getRefs(), commit.getId(), factory, root); for (VcsRef ref : refsInRecord) { @@ -904,7 +885,7 @@ public class GitHistoryUtils { if (factory == null) { return Collections.emptyList(); } - return loadDetails(project, root, false, true, record -> createCommit(project, root, record, factory), parameters); + return collectDetails(project, root, false, true, record -> createCommit(project, root, record, factory), parameters); } @NotNull @@ -937,26 +918,48 @@ public class GitHistoryUtils { } @NotNull - public static List loadDetails(@NotNull final Project project, - @NotNull final VirtualFile root, - boolean withRefs, - boolean withChanges, - @NotNull NullableFunction converter, - String... parameters) + public static List collectDetails(@NotNull Project project, + @NotNull VirtualFile root, + boolean withRefs, + boolean withChanges, + @NotNull NullableFunction converter, + String... parameters) throws VcsException { + + List commits = ContainerUtil.newArrayList(); + + loadDetails(project, root, withRefs, withChanges, record -> commits.add(converter.fun(record)), parameters); + + return commits; + } + + public static void loadDetails(@NotNull Project project, + @NotNull VirtualFile root, + @NotNull Consumer commitConsumer, + @NotNull String... parameters) throws VcsException { + final VcsLogObjectsFactory factory = getObjectsFactoryWithDisposeCheck(project); + if (factory == null) { + return; + } + + loadDetails(project, root, false, true, record -> commitConsumer.consume(createCommit(project, root, record, factory)), parameters); + } + + public static void loadDetails(@NotNull Project project, + @NotNull VirtualFile root, + boolean withRefs, + boolean withChanges, + @NotNull Consumer converter, + String... parameters) throws VcsException { GitLineHandler h = new GitLineHandler(project, root, GitCommand.LOG); GitLogParser parser = createParserForDetails(h, project, withRefs, withChanges, parameters); - List commits = ContainerUtil.newArrayList(); - StopWatch sw = StopWatch.start("loading details"); - processHandlerOutputByLine(h, parser, record -> commits.add(converter.fun(record))); + processHandlerOutputByLine(h, parser, record -> converter.consume(record)); sw.report(); - - return commits; } @NotNull diff --git a/plugins/git4idea/src/git4idea/log/GitLogProvider.java b/plugins/git4idea/src/git4idea/log/GitLogProvider.java index 04712fde06df..9aaa0dc9dc22 100644 --- a/plugins/git4idea/src/git4idea/log/GitLogProvider.java +++ b/plugins/git4idea/src/git4idea/log/GitLogProvider.java @@ -319,7 +319,25 @@ public class GitLogProvider implements VcsLogProvider { return; } - GitHistoryUtils.loadAllDetails(myProject, root, commitConsumer); + GitHistoryUtils.loadDetails(myProject, root, commitConsumer, ArrayUtil.toStringArray(GitHistoryUtils.LOG_ALL)); + } + + @Override + public void readFullDetails(@NotNull VirtualFile root, + @NotNull List hashes, + @NotNull Consumer commitConsumer) throws VcsException { + if (!isRepositoryReady(root)) { + return; + } + + VcsFileUtil + .foreachChunk(hashes, 1, hashesChunk -> { + String noWalk = GitVersionSpecialty.NO_WALK_UNSORTED.existsIn(myVcs.getVersion()) ? "--no-walk=unsorted" : "--no-walk"; + List parameters = new ArrayList<>(); + parameters.add(noWalk); + parameters.addAll(hashesChunk); + GitHistoryUtils.loadDetails(myProject, root, commitConsumer, ArrayUtil.toStringArray(parameters)); + }); } @NotNull @@ -337,26 +355,6 @@ public class GitLogProvider implements VcsLogProvider { }); } - @NotNull - @Override - public List readFullDetails(@NotNull final VirtualFile root, @NotNull List hashes) - throws VcsException { - //noinspection Convert2Lambda - return VcsFileUtil - .foreachChunk(hashes, new ThrowableNotNullFunction, List, VcsException>() { - @NotNull - @Override - public List fun(@NotNull List hashes) throws VcsException { - String noWalk = GitVersionSpecialty.NO_WALK_UNSORTED.existsIn(myVcs.getVersion()) ? "--no-walk=unsorted" : "--no-walk"; - List params = new ArrayList<>(); - params.add(noWalk); - params.addAll(hashes); - - return GitHistoryUtils.history(myProject, root, ArrayUtil.toStringArray(params)); - } - }); - } - @NotNull private Set readBranches(@NotNull GitRepository repository) { StopWatch sw = StopWatch.start("readBranches in " + repository.getRoot().getName()); @@ -450,7 +448,8 @@ public class GitLogProvider implements VcsLogProvider { List authors = ContainerUtil.map(filterCollection.getUserFilter().getUserNames(root), UserNameRegex.BASIC_INSTANCE); if (GitVersionSpecialty.LOG_AUTHOR_FILTER_SUPPORTS_VERTICAL_BAR.existsIn(myVcs.getVersion())) { filterParameters.add(prepareParameter("author", StringUtil.join(authors, "\\|"))); - } else { + } + else { filterParameters.addAll(authors.stream().map(a -> prepareParameter("author", a)).collect(Collectors.toList())); } } diff --git a/plugins/git4idea/src/git4idea/reset/GitUncommitAction.java b/plugins/git4idea/src/git4idea/reset/GitUncommitAction.java index 8d1c8a90e1e8..5ba7433398fa 100644 --- a/plugins/git4idea/src/git4idea/reset/GitUncommitAction.java +++ b/plugins/git4idea/src/git4idea/reset/GitUncommitAction.java @@ -218,7 +218,9 @@ public class GitUncommitAction extends DumbAwareAction { VirtualFile root = commit.getRoot(); VcsFullCommitDetails details = getChangesFromCache(data, hash, root); if (details == null) { - details = data.getLogProvider(root).readFullDetails(root, singletonList(hash.asString())).get(0); + Ref ref = new Ref<>(); + data.getLogProvider(root).readFullDetails(root, singletonList(hash.asString()), ref::set); + details = ref.get(); } return details.getChanges(); } diff --git a/plugins/git4idea/tests/git4idea/history/GitHistoryUtilsTest.java b/plugins/git4idea/tests/git4idea/history/GitHistoryUtilsTest.java index 98fe9905781f..8668fcfbf5eb 100644 --- a/plugins/git4idea/tests/git4idea/history/GitHistoryUtilsTest.java +++ b/plugins/git4idea/tests/git4idea/history/GitHistoryUtilsTest.java @@ -399,7 +399,7 @@ public class GitHistoryUtilsTest extends GitSingleRepoTest { touch("file.txt", "content"); addCommit(message); - GitHistoryUtils.loadAllDetails(myProject, myRepo.getRoot(), details::add); + GitHistoryUtils.loadDetails(myProject, myRepo.getRoot(), details::add); VcsFullCommitDetails lastCommit = ContainerUtil.getFirstItem(details); assertNotNull(lastCommit); @@ -423,7 +423,7 @@ public class GitHistoryUtilsTest extends GitSingleRepoTest { expected = ContainerUtil.reverse(expected); List actualMessages = - GitHistoryUtils.loadDetails(myProject, myRepo.getRoot(), true, false, GitLogRecord::getHash, "--max-count=" + commitCount); + GitHistoryUtils.collectDetails(myProject, myRepo.getRoot(), true, false, GitLogRecord::getHash, "--max-count=" + commitCount); assertEquals(expected, actualMessages); } diff --git a/plugins/git4idea/tests/git4idea/log/GitLogProviderTest.java b/plugins/git4idea/tests/git4idea/log/GitLogProviderTest.java index 620c843ab7cc..21a48d40c5f5 100644 --- a/plugins/git4idea/tests/git4idea/log/GitLogProviderTest.java +++ b/plugins/git4idea/tests/git4idea/log/GitLogProviderTest.java @@ -16,13 +16,10 @@ package git4idea.log; import com.intellij.openapi.components.ServiceManager; -import com.intellij.openapi.util.Condition; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vcs.VcsException; -import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.ArrayUtil; import com.intellij.util.CollectConsumer; -import com.intellij.util.Consumer; import com.intellij.util.Function; import com.intellij.util.containers.ContainerUtil; import com.intellij.vcs.log.*; @@ -179,12 +176,13 @@ public class GitLogProviderTest extends GitSingleRepoTest { final List hashes = ContainerUtil.newArrayList(); myLogProvider.readAllHashes(myProjectRoot, timedVcsCommit -> hashes.add(timedVcsCommit.getId().asString())); - List fullDetails = myLogProvider.readFullDetails(myProjectRoot, hashes); + List result = ContainerUtil.newArrayList(); + myLogProvider.readFullDetails(myProjectRoot, hashes, result::add); // we do not check for changes here final Function shortDetailsToString = getShortDetailsToString(); Function metadataToString = details -> shortDetailsToString.fun(details) + "\n" + details.getFullMessage(); - assertOrderedEquals(ContainerUtil.map(fullDetails, metadataToString), ContainerUtil.map(log, metadataToString)); + assertOrderedEquals(ContainerUtil.map(result, metadataToString), ContainerUtil.map(log, metadataToString)); } @NotNull diff --git a/plugins/hg4idea/src/org/zmlx/hg4idea/log/HgLogProvider.java b/plugins/hg4idea/src/org/zmlx/hg4idea/log/HgLogProvider.java index 119ba11d3f11..36b0f8226d4d 100644 --- a/plugins/hg4idea/src/org/zmlx/hg4idea/log/HgLogProvider.java +++ b/plugins/hg4idea/src/org/zmlx/hg4idea/log/HgLogProvider.java @@ -69,9 +69,9 @@ public class HgLogProvider implements VcsLogProvider { @NotNull @Override public DetailedLogData readFirstBlock(@NotNull VirtualFile root, - @NotNull Requirements requirements) throws VcsException { + @NotNull Requirements requirements) throws VcsException { List commits = HgHistoryUtil.loadMetadata(myProject, root, requirements.getCommitCount(), - Collections.emptyList()); + Collections.emptyList()); return new LogDataImpl(readAllRefs(root), commits); } @@ -88,7 +88,14 @@ public class HgLogProvider implements VcsLogProvider { } @Override - public void readAllFullDetails(@NotNull final VirtualFile root, @NotNull Consumer commitConsumer) + public void readAllFullDetails(@NotNull VirtualFile root, @NotNull Consumer commitConsumer) throws VcsException { + readFullDetails(root, ContainerUtil.newArrayList(), commitConsumer); + } + + @Override + public void readFullDetails(@NotNull VirtualFile root, + @NotNull List hashes, + @NotNull Consumer commitConsumer) throws VcsException { // this method currently is very slow and time consuming // so indexing is not to be used for mercurial for now @@ -97,8 +104,8 @@ public class HgLogProvider implements VcsLogProvider { final HgVersion version = hgvcs.getVersion(); final String[] templates = HgBaseLogParser.constructFullTemplateArgument(true, version); - HgCommandResult logResult = - HgHistoryUtil.getLogResult(myProject, root, version, -1, ContainerUtil.newArrayList(), HgChangesetUtil.makeTemplate(templates)); + HgCommandResult logResult = HgHistoryUtil.getLogResult(myProject, root, version, -1, + HgHistoryUtil.prepareHashes(hashes), HgChangesetUtil.makeTemplate(templates)); if (logResult == null) return; if (!logResult.getErrorLines().isEmpty()) throw new VcsException(logResult.getRawError()); HgHistoryUtil.createFullCommitsFromResult(myProject, root, logResult, version, false).forEach(commitConsumer::consume); @@ -148,7 +155,7 @@ public class HgLogProvider implements VcsLogProvider { for (HgNameWithHashInfo bookmarkInfo : bookmarks) { refs.add(myVcsObjectsFactory.createRef(bookmarkInfo.getHash(), bookmarkInfo.getName(), - HgRefManager.BOOKMARK, root)); + HgRefManager.BOOKMARK, root)); } String currentRevision = repository.getCurrentRevision(); if (currentRevision != null) { // null => fresh repository @@ -163,7 +170,7 @@ public class HgLogProvider implements VcsLogProvider { } for (HgNameWithHashInfo localTagInfo : localTags) { refs.add(myVcsObjectsFactory.createRef(localTagInfo.getHash(), localTagInfo.getName(), - HgRefManager.LOCAL_TAG, root)); + HgRefManager.LOCAL_TAG, root)); } for (HgNameWithHashInfo mqPatchRef : mqAppliedPatches) { refs.add(myVcsObjectsFactory.createRef(mqPatchRef.getHash(), mqPatchRef.getName(),