CommitGraphWriter: Unnest generation-number progress

The ProgressMonitor task to track the calculation of generation
numbers is nested inside the task that follows the writing of all
lines in the commit-graph. ProgressMonitor doesn't support nested
tasks and this confuses the counting.

Move the start/end of the "writing commit graph" task to the
writeCommitData section, after calculating the generation
numbers. Make that task track by commits instead of by lines.

Moving the start/end of the progress task to the chunk-writing
functions is clearer and easier to extend.

Logging of GC before:
Writing out commit-graph in 3 passes:  51% ( 9807/19358)
Computing commit-graph generation numbers: 100% (9551/9551)

Logging of GC after:
Computing commit-graph generation numbers: 100% (9551/9551)
Writing out commit-graph: 100% (9551/9551)

Change-Id: I87d69c06c9a3c7e75be12b6f0d1a63b5924e298a

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java
index 5040a3b6ad86536bd301a18006274b174e16b741..de3610563d3441a757ef3000de85c62a0a730320 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java
@@ -14,6 +14,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayOutputStream;
@@ -31,6 +32,7 @@ import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.revwalk.RevBlob;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -135,6 +137,56 @@ public class CommitGraphWriterTest extends RepositoryTestCase {
                                NB.decodeInt32(data, 56));
        }
 
+       @Test
+       public void testProgressMonitor() throws Exception {
+               RevCommit root = commit();
+               RevCommit a = commit(root);
+               RevCommit b = commit(root);
+               RevCommit tip = commit(a, b);
+               Set<ObjectId> wants = Collections.singleton(tip);
+
+               NonNestedTasksProgressMonitor nonNested = new NonNestedTasksProgressMonitor();
+               GraphCommits graphCommits = GraphCommits.fromWalk(nonNested, wants,
+                               walk);
+               writer = new CommitGraphWriter(graphCommits, true);
+               writer.write(nonNested, os);
+       }
+
+       private static class NonNestedTasksProgressMonitor
+                       implements ProgressMonitor {
+
+               boolean inTask;
+
+               @Override
+               public void start(int totalTasks) {
+               }
+
+               @Override
+               public void beginTask(String title, int totalWork) {
+                       assertFalse("Previous monitoring task is not closed", inTask);
+                       inTask = true;
+               }
+
+               @Override
+               public void update(int completed) {
+               }
+
+               @Override
+               public void endTask() {
+                       assertTrue("Closing task that wasn't started", inTask);
+                       inTask = false;
+               }
+
+               @Override
+               public boolean isCancelled() {
+                       return false;
+               }
+
+               @Override
+               public void showDuration(boolean enabled) {
+               }
+       }
+
        static HashSet<String> changedPathStrings(byte[] data) {
                int oidf_offset = -1;
                int bidx_offset = -1;

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 84f5a7638c9ff2342cf72446d42afd09be9f9b41..6bd4cb136ca32002c45ee9598de28e6612721bac 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -886,7 +886,7 @@ writerAlreadyInitialized=Writer already initialized
 writeTimedOut=Write timed out after {0} ms
 writingNotPermitted=Writing not permitted
 writingNotSupported=Writing {0} not supported.
-writingOutCommitGraph=Writing out commit-graph in {0} passes
+writingOutCommitGraph=Writing out commit-graph
 writingObjects=Writing objects
 wrongDecompressedLength=wrong decompressed length
 wrongRepositoryState=Wrong Repository State: {0}

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
index f72c1eaa8c977f0e693937be8120c607d6c52282..1c5f5b85219b72be39b5372cdd196a8aa2d4e006 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
@@ -31,7 +31,6 @@ import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
-import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
@@ -131,13 +130,6 @@ public class CommitGraphWriter {
                chunks = Collections.unmodifiableList(chunks);
 
                long expectedSize = calculateExpectedSize(chunks);
-               long writeCount = 256L + 2 * graphCommits.size()
-                               + graphCommits.getExtraEdgeCnt();
-               monitor.beginTask(
-                               MessageFormat.format(JGitText.get().writingOutCommitGraph,
-                                               Integer.valueOf(chunks.size())),
-                               (int) writeCount);
-
                try (CancellableDigestOutputStream out = new CancellableDigestOutputStream(
                                monitor, commitGraphStream)) {
                        writeHeader(out, chunks.size());
@@ -153,8 +145,6 @@ public class CommitGraphWriter {
                } catch (InterruptedIOException e) {
                        throw new IOException(JGitText.get().commitGraphWritingCancelled,
                                        e);
-               } finally {
-                       monitor.endTask();
                }
                return Stats.from(bloomFilterChunks);
        }
@@ -290,6 +280,8 @@ public class CommitGraphWriter {
        private void writeCommitData(ProgressMonitor monitor,
                        CancellableDigestOutputStream out) throws IOException {
                int[] generations = computeGenerationNumbers(monitor);
+               monitor.beginTask(JGitText.get().writingOutCommitGraph,
+                               graphCommits.size());
                int num = 0;
                byte[] tmp = new byte[hashsz + COMMIT_DATA_WIDTH];
                int i = 0;
@@ -330,6 +322,7 @@ public class CommitGraphWriter {
                        out.getWriteMonitor().update(1);
                        i++;
                }
+               monitor.endTask();
        }
 
        private int[] computeGenerationNumbers(ProgressMonitor monitor)