Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2018-07-17 06:30:24 -0400
committerThomas Wolf2018-07-17 06:34:02 -0400
commitcdbe0608d9756bdcf17fcb8eee245e911fe7ec96 (patch)
tree78b73c402bf2df39b75fd8c438017c5da515a6c1 /org.eclipse.egit.ui/src/org/eclipse/egit/ui
parent9cf5c4a0fa4718d2070b1dac3b7bc7eb17784de2 (diff)
downloadegit-cdbe0608d9756bdcf17fcb8eee245e911fe7ec96.tar.gz
egit-cdbe0608d9756bdcf17fcb8eee245e911fe7ec96.tar.xz
egit-cdbe0608d9756bdcf17fcb8eee245e911fe7ec96.zip
Avoid that loading the history blocks updating the commit table
Somehow setting the input on the commit graph table got blocked on the monitor of the SWTCommitList. The only other place where that monitor can be held is in the GenerateHistoryJob itself. However, CommitGraphTable.setInput() is always called not only with that SWTCommitList but also its contents copied into an array. Thus we can use this array copy instead of the list itself to get the contents. On the array, no synchronization is needed. That also means that inside the GenerateHistoryJob no synchronization on that list is needed anymore, since only that job accesses it. Bug: 485743 Change-Id: I77007b159d8b3de0a346b7cea69599fd61d80585 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Diffstat (limited to 'org.eclipse.egit.ui/src/org/eclipse/egit/ui')
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java21
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GenerateHistoryJob.java38
2 files changed, 27 insertions, 32 deletions
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java
index 20e644cca..4c24d641f 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java
@@ -379,11 +379,12 @@ class CommitGraphTable {
}
highlight = hFlag;
allCommits = list;
- int newAllCommitsLength = allCommits.size();
+ int newAllCommitsLength = asArray == null ? 0 : asArray.length;
table.setInput(asArray);
- if (asArray != null && asArray.length > 0) {
- if (oldList != list || allCommitsLength < newAllCommitsLength)
- initCommitsMap();
+ if (newAllCommitsLength > 0) {
+ if (oldList != list || allCommitsLength < newAllCommitsLength) {
+ initCommitsMap(asArray);
+ }
} else {
table.getTable().deselectAll();
// Fire an event
@@ -402,14 +403,12 @@ class CommitGraphTable {
this.input = input;
}
- private void initCommitsMap() {
+ private void initCommitsMap(SWTCommit[] asArray) {
commitsMap = new HashMap<>();
- // ensure that filling (GenerateHistoryJob) and reading (here)
- // the commit list is thread safe
- synchronized (allCommits) {
- for (PlotCommit commit : allCommits)
- if (commit != null)
- commitsMap.put(commit.getId().name(), commit);
+ for (SWTCommit commit : asArray) {
+ if (commit != null) {
+ commitsMap.put(commit.getId().name(), commit);
+ }
}
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GenerateHistoryJob.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GenerateHistoryJob.java
index a45a30b46..b67e00495 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GenerateHistoryJob.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GenerateHistoryJob.java
@@ -25,11 +25,11 @@ import org.eclipse.egit.ui.JobFamilies;
import org.eclipse.egit.ui.UIPreferences;
import org.eclipse.egit.ui.internal.UIText;
import org.eclipse.egit.ui.internal.trace.GitTraceLocation;
-import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jface.resource.ResourceManager;
+import org.eclipse.jgit.annotations.NonNull;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.osgi.util.NLS;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Display;
@@ -92,28 +92,24 @@ class GenerateHistoryJob extends Job {
GitTraceLocation.getTrace().trace(
GitTraceLocation.HISTORYVIEW.getLocation(),
"Filling commit list"); //$NON-NLS-1$
- // ensure that filling (here) and reading (CommitGraphTable)
- // the commit list is thread safe
- synchronized (loadedCommits) {
- if (commitToLoad != null) {
- loadedCommits.fillTo(commitToLoad, maxCommits);
- commitToShow = commitToLoad;
- commitToLoad = null;
- boolean commitFound = false;
- for (RevCommit commit : loadedCommits) {
- if (commit.getId().equals(commitToShow.getId())) {
- commitFound = true;
- break;
- }
- }
- commitNotFound = !commitFound;
- } else {
- loadedCommits.fillTo(oldsz + BATCH_SIZE - 1);
- if (oldsz == loadedCommits.size()) {
- forcedRedrawsAfterListIsCompleted++;
+ if (commitToLoad != null) {
+ loadedCommits.fillTo(commitToLoad, maxCommits);
+ commitToShow = commitToLoad;
+ commitToLoad = null;
+ boolean commitFound = false;
+ for (RevCommit commit : loadedCommits) {
+ if (commit.getId().equals(commitToShow.getId())) {
+ commitFound = true;
break;
}
}
+ commitNotFound = !commitFound;
+ } else {
+ loadedCommits.fillTo(oldsz + BATCH_SIZE - 1);
+ if (oldsz == loadedCommits.size()) {
+ forcedRedrawsAfterListIsCompleted++;
+ break;
+ }
}
if (monitor.isCanceled())
return Status.CANCEL_STATUS;

Back to the top