From 09803522eb830b9510499bb5de3e7a38f5649563 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Mon, 4 Mar 2024 21:23:51 +0100 Subject: [PATCH] [vcs-log-graph] use all branch heads for the graph layout Head commits are used to calculate layout indexes, which are used for sorting nodes and edges and coloring. However, before this commit only graph heads, and not branch heads were used. Because of this, some commits belonging to important branches were not laid out or colored properly, just because the important branch head was contained in some other less important branch. GitOrigin-RevId: 1cd228ff6107d0ca03d5f3172f9940e0d55d9e4f --- .../graph/impl/facade/PermanentGraphImpl.kt | 4 +- .../impl/facade/bek/BekBranchCreator.java | 9 ++-- .../impl/permanent/GraphLayoutBuilder.kt | 48 ++++++++++++------- .../permanent/PermanentCommitsInfoImpl.kt | 13 +++-- .../intellij/vcs/log/data/CompressedRefs.java | 4 ++ .../com/intellij/vcs/log/data/RefsModel.kt | 11 ++++- 6 files changed, 59 insertions(+), 30 deletions(-) diff --git a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/PermanentGraphImpl.kt b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/PermanentGraphImpl.kt index 7d8a083e9e83..7ebf26aea73d 100644 --- a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/PermanentGraphImpl.kt +++ b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/PermanentGraphImpl.kt @@ -163,8 +163,8 @@ class PermanentGraphImpl private constructor(private val permane val idsGenerator = NotLoadedCommitsIdsGenerator() val linearGraph = PermanentLinearGraphBuilder.newInstance(graphCommits).build(idsGenerator) val permanentCommitsInfo = PermanentCommitsInfoImpl.newInstance(graphCommits, idsGenerator.notLoadedCommits) - - val permanentGraphLayout = GraphLayoutBuilder.build(linearGraph) { nodeIndex1: Int, nodeIndex2: Int -> + val branchIndexes = permanentCommitsInfo.convertToNodeIds(branchesCommitId, true) + val permanentGraphLayout = GraphLayoutBuilder.build(linearGraph, branchIndexes) { nodeIndex1: Int, nodeIndex2: Int -> val commitId1 = permanentCommitsInfo.getCommitId(nodeIndex1) val commitId2 = permanentCommitsInfo.getCommitId(nodeIndex2) headCommitsComparator.compare(commitId1, commitId2) diff --git a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/bek/BekBranchCreator.java b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/bek/BekBranchCreator.java index b7024260d88f..202508ea40bc 100644 --- a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/bek/BekBranchCreator.java +++ b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/bek/BekBranchCreator.java @@ -1,7 +1,6 @@ -// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.vcs.log.graph.impl.facade.bek; -import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Pair; import com.intellij.vcs.log.graph.api.LinearGraph; import com.intellij.vcs.log.graph.impl.permanent.GraphLayoutImpl; @@ -18,7 +17,6 @@ import static com.intellij.vcs.log.graph.utils.LinearGraphUtils.getDownNodes; import static com.intellij.vcs.log.graph.utils.LinearGraphUtils.getUpNodes; class BekBranchCreator { - private final static Logger LOG = Logger.getInstance(BekBranchCreator.class); @NotNull private final LinearGraph myPermanentGraph; @NotNull private final GraphLayoutImpl myGraphLayout; @NotNull private final Flags myDoneNodes; @@ -36,6 +34,7 @@ class BekBranchCreator { List bekBranches = new ArrayList<>(); for (int headNode : myGraphLayout.getHeadNodeIndex()) { + if (myDoneNodes.get(headNode)) continue; List nextBranch = createNextBranch(headNode); bekBranches.add(new BekBranch(myPermanentGraph, nextBranch)); } @@ -43,10 +42,8 @@ class BekBranchCreator { } public List createNextBranch(int headNode) { - final List nodeIndexes = new ArrayList<>(); - - LOG.assertTrue(!myDoneNodes.get(headNode)); myDoneNodes.set(headNode, true); + List nodeIndexes = new ArrayList<>(); nodeIndexes.add(headNode); final int startLayout = myGraphLayout.getLayoutIndex(headNode); diff --git a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/GraphLayoutBuilder.kt b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/GraphLayoutBuilder.kt index 2418b0428edb..08a7e25afc20 100644 --- a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/GraphLayoutBuilder.kt +++ b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/GraphLayoutBuilder.kt @@ -16,35 +16,47 @@ object GraphLayoutBuilder { @JvmStatic fun build(graph: LinearGraph, comparator: IntComparator): GraphLayoutImpl { - val heads = getSortedHeads(graph, comparator) - - val layoutIndex = IntArray(graph.nodesCount()) - val startLayoutIndexForHead = IntArray(heads.size) - var currentLayoutIndex = 1 - for (i in heads.indices) { - startLayoutIndexForHead[i] = currentLayoutIndex - currentLayoutIndex = dfs(graph, heads.getInt(i), currentLayoutIndex, layoutIndex) - } - return GraphLayoutImpl(layoutIndex, heads, startLayoutIndexForHead) + return build(graph, emptySet(), comparator) } - private fun getSortedHeads(graph: LinearGraph, comparator: IntComparator): IntList { - val heads: IntList = IntArrayList() - for (i in 0 until graph.nodesCount()) { - if (LinearGraphUtils.getUpNodes(graph, i).isEmpty()) { - heads.add(i) - } + @JvmStatic + fun build(graph: LinearGraph, branches: Set, comparator: IntComparator): GraphLayoutImpl { + val allHeads = branches + graph.getHeads() + val sortedHeads = IntArrayList(allHeads).sortCatching(comparator) + + val layoutIndex = IntArray(graph.nodesCount()) + val startLayoutIndexForHead = IntArray(sortedHeads.size) + var currentLayoutIndex = 1 + for (i in sortedHeads.indices) { + startLayoutIndexForHead[i] = currentLayoutIndex + currentLayoutIndex = dfs(graph, sortedHeads.getInt(i), currentLayoutIndex, layoutIndex) } + return GraphLayoutImpl(layoutIndex, sortedHeads, startLayoutIndexForHead) + } + + /** + * Performs sorting, while catching exceptions from the comparator. + */ + private fun IntList.sortCatching(comparator: IntComparator): IntList { try { - heads.sort(comparator) + sort(comparator) } catch (pce: ProcessCanceledException) { throw pce } catch (e: Exception) { - // protection against possible comparator flaws LOG.error(e) } + return this + } + + private fun LinearGraph.getHeads(): IntList { + val heads = IntArrayList() + for (i in 0 until nodesCount()) { + if (LinearGraphUtils.getUpNodes(this, i).isEmpty()) { + heads.add(i) + } + } return heads } diff --git a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/PermanentCommitsInfoImpl.kt b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/PermanentCommitsInfoImpl.kt index e864868d83df..40390f9a37ee 100644 --- a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/PermanentCommitsInfoImpl.kt +++ b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/permanent/PermanentCommitsInfoImpl.kt @@ -9,6 +9,7 @@ import com.intellij.vcs.log.graph.utils.impl.CompressedIntList import com.intellij.vcs.log.graph.utils.impl.IntTimestampGetter import it.unimi.dsi.fastutil.ints.Int2ObjectMap import it.unimi.dsi.fastutil.ints.IntOpenHashSet +import it.unimi.dsi.fastutil.ints.IntSet import java.util.* class PermanentCommitsInfoImpl private constructor(val timestampGetter: TimestampGetter, @@ -50,6 +51,10 @@ class PermanentCommitsInfoImpl private constructor(val timestamp } override fun convertToNodeIds(commitIds: Collection): Set { + return convertToNodeIds(commitIds, false) + } + + internal fun convertToNodeIds(commitIds: Collection, skipNotLoadedCommits: Boolean): IntSet { val result = IntOpenHashSet() for (i in commitIdIndexes.indices) { val commitId = commitIdIndexes[i] @@ -57,9 +62,11 @@ class PermanentCommitsInfoImpl private constructor(val timestamp result.add(i) } } - for (entry in notLoadedCommits.int2ObjectEntrySet()) { - if (commitIds.contains(entry.value)) { - result.add(entry.intKey) + if (!skipNotLoadedCommits) { + for (entry in notLoadedCommits.int2ObjectEntrySet()) { + if (commitIds.contains(entry.value)) { + result.add(entry.intKey) + } } } return result diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CompressedRefs.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CompressedRefs.java index 7fd82c1f2f37..184662d99b37 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CompressedRefs.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/CompressedRefs.java @@ -106,4 +106,8 @@ public final class CompressedRefs { myTags.keySet().intStream().forEach(result::add); return result; } + + @NotNull Int2ObjectMap> getBranches() { + return myBranches; + } } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/RefsModel.kt b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/RefsModel.kt index 9dcfb5cf5db5..092fd2d72a7f 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/RefsModel.kt +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/RefsModel.kt @@ -10,6 +10,8 @@ import com.intellij.vcs.log.VcsLogRefs import com.intellij.vcs.log.VcsRef import it.unimi.dsi.fastutil.ints.Int2ObjectMap import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap +import it.unimi.dsi.fastutil.ints.IntOpenHashSet +import java.util.function.IntConsumer import java.util.stream.Collectors import java.util.stream.Stream @@ -65,7 +67,14 @@ class RefsModel(val allRefsByRoot: Map, private val providers: Map): RefsModel { val refsModel = RefsModel(refs, storage, providers) - storage.getCommitIds(heads).forEach { (head, commitId) -> refsModel.updateCacheForHead(head, commitId.root) } + val remainingHeads = IntOpenHashSet(heads) + refs.forEach { (root, refsForRoot) -> + refsForRoot.branches.keys.forEach(IntConsumer { commit -> + refsModel.updateCacheForHead(commit, root) + remainingHeads.remove(commit) + }) + } + storage.getCommitIds(remainingHeads).forEach { (head, commitId) -> refsModel.updateCacheForHead(head, commitId.root) } return refsModel }