From 2011fe06d2cbc15159342b263704f95a39f937c4 Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Fri, 6 Jan 2023 14:14:48 -0700 Subject: [PATCH 1/8] FileSnapshotTest: Add more MISSING_FILE coverage Add a couple tests that confirm what the docs say about isModified() and equals(MISSING_FILE) behavior. Change-Id: I6093040ba3594934c3270331405a44b2634b97c5 Signed-off-by: Nasser Grainawi --- .../internal/storage/file/FileSnapshotTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java index 5e87b8f59..12773c240 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java @@ -209,6 +209,20 @@ public void fileSnapshotEquals() throws Exception { assertTrue(fs2.equals(fs1)); } + @Test + public void snapshotAndFileMissingIsNotModified() throws Exception { + File doesNotExist = trash.resolve("DOES_NOT_EXIST").toFile(); + FileSnapshot missing = FileSnapshot.save(doesNotExist); + assertFalse(missing.isModified(doesNotExist)); + } + + @Test + public void missingFileEquals() throws Exception { + FileSnapshot missing = FileSnapshot.save( + trash.resolve("DOES_NOT_EXIST").toFile()); + assertTrue(missing.equals(FileSnapshot.MISSING_FILE)); + } + @SuppressWarnings("boxing") @Test public void detectFileModified() throws IOException { From cd3fc7a2995c06cf2425f51758094e039c938559 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 18 Jan 2023 17:39:19 +0100 Subject: [PATCH 2/8] Speedup GC listing objects referenced from reflogs GC needs to get a ReflogReader for all existing refs to list all objects referenced from reflogs. The existing Repository#getReflogReader method accepts the ref name and then resolves the Ref to create a ReflogReader. GC calling that for a huge number of Refs one by one is very slow. GC first gets all Refs in bulk and then calls getReflogReader for each of them. Fix this by adding another getReflogReader method to Repository which accepts a Ref directly. This speeds up running JGit gc on a mirror clone of the Gerrit repository from 15:36 min to 1:08 min. The repository used in this test had 45k refs, 275k commits and 1.2m git objects. Change-Id: I474897fdc6652923e35d461c065a29f54d9949f4 --- .../.settings/.api_filters | 17 ---------- org.eclipse.jgit/.settings/.api_filters | 32 +++++++------------ .../internal/storage/file/FileRepository.java | 7 ++++ .../jgit/internal/storage/file/GC.java | 5 +-- .../src/org/eclipse/jgit/lib/Repository.java | 16 ++++++++++ 5 files changed, 36 insertions(+), 41 deletions(-) delete mode 100644 org.eclipse.jgit.http.server/.settings/.api_filters diff --git a/org.eclipse.jgit.http.server/.settings/.api_filters b/org.eclipse.jgit.http.server/.settings/.api_filters deleted file mode 100644 index 951a53bf3..000000000 --- a/org.eclipse.jgit.http.server/.settings/.api_filters +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - - - - - - - - diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 87deace39..27b322c91 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -1,5 +1,13 @@ + + + + + + + + @@ -8,11 +16,11 @@ - - + + - - + + @@ -24,22 +32,6 @@ - - - - - - - - - - - - - - - - diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java index fecced1ae..90ee8def5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java @@ -31,6 +31,7 @@ import java.util.Objects; import java.util.Set; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.attributes.AttributesNode; @@ -525,6 +526,12 @@ public ReflogReader getReflogReader(String refName) throws IOException { return new ReflogReaderImpl(this, ref.getName()); } + @Override + public @NonNull ReflogReader getReflogReader(@NonNull Ref ref) + throws IOException { + return new ReflogReaderImpl(this, ref.getName()); + } + /** {@inheritDoc} */ @Override public AttributesNodeProvider createAttributesNodeProvider() { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index 40c075ec5..a14bb411f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -1019,10 +1019,7 @@ private void deleteTempPacksIdx() { * @throws IOException */ private Set listRefLogObjects(Ref ref, long minTime) throws IOException { - ReflogReader reflogReader = repo.getReflogReader(ref.getName()); - if (reflogReader == null) { - return Collections.emptySet(); - } + ReflogReader reflogReader = repo.getReflogReader(ref); List rlEntries = reflogReader .getReverseEntries(); if (rlEntries == null || rlEntries.isEmpty()) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index 1e8a6c917..d3b3c6e8a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -1691,6 +1691,22 @@ public void setGitwebDescription(@Nullable String description) public abstract ReflogReader getReflogReader(String refName) throws IOException; + /** + * Get the reflog reader. Subclasses should override this method and provide + * a more efficient implementation. + * + * @param ref + * a Ref + * @return a {@link org.eclipse.jgit.lib.ReflogReader} for the supplied ref, + * or {@code null} if the ref does not exist. + * @throws IOException + * @since 5.13.2 + */ + public @Nullable ReflogReader getReflogReader(@NonNull Ref ref) + throws IOException { + return getReflogReader(ref.getName()); + } + /** * Return the information stored in the file $GIT_DIR/MERGE_MSG. In this * file operations triggering a merge will store a template for the commit From 611412a05528ba5898cf948f4d3959f1fbc4170a Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 16 Jan 2023 21:58:56 +0100 Subject: [PATCH 3/8] BatchingProgressMonitor: avoid int overflow when computing percentage When cloning huge repositories I observed percentage of object counts turning negative. This happened if lastWork * 100 exceeded Integer.MAX_VALUE. Change-Id: Ic5f5cf5a911a91338267aace4daba4b873ab3900 --- .../jgit/lib/TextProgressMonitorTest.java | 83 +++++++++++++++++++ .../jgit/lib/BatchingProgressMonitor.java | 6 +- 2 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/TextProgressMonitorTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/TextProgressMonitorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/TextProgressMonitorTest.java new file mode 100644 index 000000000..55ca2cdea --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/TextProgressMonitorTest.java @@ -0,0 +1,83 @@ +/* + * Copyright (C) 2023, SAP SE or an SAP affiliate company and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.lib; + +import java.io.ByteArrayOutputStream; +import java.io.OutputStreamWriter; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class TextProgressMonitorTest { + + private TextProgressMonitor m; + + private ByteArrayOutputStream buf; + + @Before + public void setup() { + buf = new ByteArrayOutputStream(); + m = new TextProgressMonitor( + new OutputStreamWriter(buf, StandardCharsets.UTF_8)); + } + + @Test + public void testSimple() throws Exception { + m.beginTask("task", 10); + for (int i = 0; i < 10; i++) { + m.update(1); + } + m.endTask(); + Assert.assertArrayEquals( + new String[] { "", "task: 10% ( 1/10)", + "task: 20% ( 2/10)", + "task: 30% ( 3/10)", + "task: 40% ( 4/10)", + "task: 50% ( 5/10)", + "task: 60% ( 6/10)", + "task: 70% ( 7/10)", + "task: 80% ( 8/10)", + "task: 90% ( 9/10)", + "task: 100% (10/10)", + "task: 100% (10/10)\n" }, + bufLines()); + } + + @Test + public void testLargeNumbers() throws Exception { + m.beginTask("task", 1_000_000_000); + for (int i = 0; i < 10; i++) { + m.update(100_000_000); + } + m.endTask(); + Assert.assertArrayEquals( + new String[] { "", + "task: 10% ( 100000000/1000000000)", + "task: 20% ( 200000000/1000000000)", + "task: 30% ( 300000000/1000000000)", + "task: 40% ( 400000000/1000000000)", + "task: 50% ( 500000000/1000000000)", + "task: 60% ( 600000000/1000000000)", + "task: 70% ( 700000000/1000000000)", + "task: 80% ( 800000000/1000000000)", + "task: 90% ( 900000000/1000000000)", + "task: 100% (1000000000/1000000000)", + "task: 100% (1000000000/1000000000)\n" }, + bufLines()); + } + + String[] bufLines() throws UnsupportedEncodingException { + String s = new String(buf.toString(StandardCharsets.UTF_8.name())); + return s.split("\r"); + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java index 2caefa4d9..49e295aed 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java @@ -176,7 +176,7 @@ void update(BatchingProgressMonitor pm, int completed) { } } else { // Display once per second or when 1% is done. - int currPercent = lastWork * 100 / totalWork; + int currPercent = Math.round(lastWork * 100F / totalWork); if (display) { pm.onUpdate(taskName, lastWork, totalWork, currPercent); output = true; @@ -201,8 +201,8 @@ void end(BatchingProgressMonitor pm) { if (totalWork == UNKNOWN) { pm.onEndTask(taskName, lastWork); } else { - int pDone = lastWork * 100 / totalWork; - pm.onEndTask(taskName, lastWork, totalWork, pDone); + int currPercent = Math.round(lastWork * 100F / totalWork); + pm.onEndTask(taskName, lastWork, totalWork, currPercent); } } if (timerFuture != null) From e4529cd39c42872e9b4f80d38659f9de37956634 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Wed, 28 Dec 2022 01:09:52 +0000 Subject: [PATCH 4/8] PackWriterBitmapPreparer: do not include annotated tags in bitmap The annotated tags should be excluded from the bitmap associated with the heads-only packfile. However, this was not happening because of the check of exclusion of the peeled object instead of the objectId to be excluded from the bitmap. Sample use-case: refs/heads/main ^ | commit1 <-- commit2 <- annotated-tag1 <- tag1 ^ | commit0 When creating a bitmap for the above commit graph, before this change all the commits are included (3 bitmaps), which is incorrect, because all commits reachable from annotated tags should not be included. The heads-only bitmap should include only commit0 and commit1 but because PackWriterBitPreparer was checking for the peeled pointer of tag1 to be excluded (commit2) which was not found in the list of tags to exclude (annotated-tag1), the commit2 was included, even if it wasn't reachable only from the head. Add an additional check for exclusion of the original objectId for allowing the exclusion of annotated tags and their pointed commits. Add one specific test associated with an annotated tag for making sure that this use-case is covered also. Example repository benchmark for measuring the improvement: # refs: 400k (2k heads, 88k tags, 310k changes) # objects: 11M (88k of them are annotate tags) # packfiles: 2.7G Before this change: GC time: 5h clone --bare time: 7 mins After this change: GC time: 20 mins clone --bare time: 3 mins Bug: 581267 Signed-off-by: Luca Milanesio Change-Id: Iff2bfc6587153001837220189a120ead9ac649dc --- .../storage/pack/GcCommitSelectionTest.java | 34 +++++++++++++++++++ .../pack/PackWriterBitmapPreparer.java | 3 ++ 2 files changed, 37 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java index cc826c30b..55dfa697b 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java @@ -82,6 +82,40 @@ private void testBitmapSpansNoMerges(boolean withTags) throws Exception { } } + @Test + public void testBitmapDoesNotIncludeAnnotatedTags() throws Exception { + /* + * Make sure that the bitmap generated for the following commit + * graph does not include commit2 because it is not reachable by any + * heads, despite being reachable from tag1 through the annotated-tag1. + * + * refs/heads/main + * ^ + * | + * commit1 <-- commit2 <- annotated-tag1 <- tag1 + * ^ + * | + * commit0 + */ + String mainBranch = "refs/heads/main"; + BranchBuilder bb = tr.branch(mainBranch); + + String commitMsg = "commit msg"; + String fileBody = "file body"; + String tagName = "tag1"; + bb.commit().message(commitMsg + " 1").add("file1", fileBody).create(); + RevCommit commit1 = bb.commit().message(commitMsg + " 2").add("file2", fileBody).create(); + RevCommit commit2 = bb.commit().message(commitMsg + " 3").add("file3", fileBody).create(); + tr.lightweightTag(tagName, tr.tag(tagName, commit2)); + tr.branch(mainBranch).update(commit1); + + gc.setExpireAgeMillis(0); + gc.gc(); + + // Create only 2 bitmaps, for commit0 and commit1, excluding commit2 + assertEquals(2, gc.getStatistics().numberOfBitmaps); + } + @Test public void testBitmapSpansWithMerges() throws Exception { /* diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java index f1ede2acf..9f2b4d9c8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java @@ -408,6 +408,9 @@ private CommitSelectionHelper captureOldAndNewCommits(RevWalk rw, List newWantsByNewest = new ArrayList<>(want.size()); Set newWants = new HashSet<>(want.size()); for (AnyObjectId objectId : want) { + if(excludeFromBitmapSelection.contains(objectId)) { + continue; + } RevObject ro = rw.peel(rw.parseAny(objectId)); if (!(ro instanceof RevCommit) || reuse.contains(ro) || excludeFromBitmapSelection.contains(ro)) { From ad977f157242d4d6b5ea4c45b2aa0c15d20b58ae Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Tue, 20 Dec 2022 21:50:19 +0000 Subject: [PATCH 5/8] Allow the exclusions of refs prefixes from bitmap When running a GC.repack() against a repository with over one thousands of refs/heads and tens of millions of ObjectIds, the calculation of all bitmaps associated with all the refs would result in an unreasonable big file that would take up to several hours to compute. Test scenario: repo with 2500 heads / 10M obj Intel Xeon E5-2680 2.5GHz Before this change: 20 mins After this change and 2300 heads excluded: 10 mins (90s for bitmap) Having such a large bitmap file is also slow in the runtime processing and have negligible or even negative benefits, because the time lost in reading and decompressing the bitmap in memory would not be compensated by the time saved by using it. It is key to preserve the bitmaps for those refs that are mostly used in clone/fetch and give the ability to exlude some refs prefixes that are known to be less frequently accessed, even though they may actually be actively written. Example: Gerrit sandbox branches may even be actively used and selected automatically because its commits are very recent, however, they may bloat the bitmap, making it ineffective. A mono-repo with tens of thousands of developers may have a relatively small number of active branches where the CI/CD jobs are continuously fetching/cloning the code. However, because Gerrit allows the use of sandbox branches, the total number of refs/heads may be even tens to hundred thousands. Change-Id: I466dcde69fa008e7f7785735c977f6e150e3b644 Signed-off-by: Luca Milanesio --- Documentation/config-options.md | 1 + .../storage/pack/GcCommitSelectionTest.java | 18 +++++++++ .../jgit/internal/storage/file/GC.java | 23 +++++++++-- .../org/eclipse/jgit/lib/ConfigConstants.java | 6 +++ .../eclipse/jgit/storage/pack/PackConfig.java | 38 +++++++++++++++++++ 5 files changed, 82 insertions(+), 4 deletions(-) diff --git a/Documentation/config-options.md b/Documentation/config-options.md index 19bcc3352..b4a0c1d98 100644 --- a/Documentation/config-options.md +++ b/Documentation/config-options.md @@ -86,6 +86,7 @@ Proxy configuration uses the standard Java mechanisms via class `java.net.ProxyS | `pack.bitmapContiguousCommitCount` | `100` | ⃞ | Count of most recent commits for which to build bitmaps. | | `pack.bitmapDistantCommitSpan` | `5000` | ⃞ | Span of commits when building bitmaps for distant history. | | `pack.bitmapExcessiveBranchCount` | `100` | ⃞ | The count of branches deemed "excessive". If the count of branches in a repository exceeds this number and bitmaps are enabled, "inactive" branches will have fewer bitmaps than "active" branches. | +| `pack.bitmapExcludedRefsPrefixes` | | ⃞ | The refs prefixes to be excluded when building bitmaps. May be specified more than once to exclude multiple prefixes. | | `pack.bitmapInactiveBranchAgeInDays` | `90` | ⃞ | Age in days that marks a branch as "inactive" for bitmap creation. | | `pack.bitmapRecentCommitCount` | `20000` | ⃞ | Count at which to switch from `bitmapRecentCommitSpan` to `bitmapDistantCommitSpan`. | | `pack.bitmapRecentCommitSpan` | `100` | ⃞ | Span of commits when building bitmaps for recent history. | diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java index 55dfa697b..190ac8b64 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java @@ -220,6 +220,24 @@ public void testBitmapsForExcessiveBranches() throws Exception { gc.getStatistics().numberOfBitmaps); } + @Test + public void testBitmapsForExcludedBranches() throws Exception { + createNewCommitOnNewBranch("main"); + createNewCommitOnNewBranch("other"); + PackConfig packConfig = new PackConfig(); + packConfig.setBitmapExcludedRefsPrefixes(new String[] { "refs/heads/other" }); + gc.setPackConfig(packConfig); + gc.gc(); + assertEquals(1, + gc.getStatistics().numberOfBitmaps); + } + + private void createNewCommitOnNewBranch(String branchName) throws Exception { + BranchBuilder bb = tr.branch("refs/heads/" + branchName); + String msg = "New branch " + branchName; + bb.commit().message(msg).add("some-filename.txt", msg).create(); + } + @Test public void testSelectionOrderingWithChains() throws Exception { /*- diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index a14bb411f..9e9765949 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -796,6 +796,10 @@ public Collection repack() throws IOException { Set tagTargets = new HashSet<>(); Set indexObjects = listNonHEADIndexObjects(); + Set refsToExcludeFromBitmap = repo.getRefDatabase() + .getRefsByPrefix(pconfig.getBitmapExcludedRefsPrefixes()) + .stream().map(Ref::getObjectId).collect(Collectors.toSet()); + for (Ref ref : refsBefore) { checkCancelled(); nonHeads.addAll(listRefLogObjects(ref, 0)); @@ -840,7 +844,7 @@ public Collection repack() throws IOException { Pack heads = null; if (!allHeadsAndTags.isEmpty()) { heads = writePack(allHeadsAndTags, PackWriter.NONE, allTags, - tagTargets, excluded); + refsToExcludeFromBitmap, tagTargets, excluded); if (heads != null) { ret.add(heads); excluded.add(0, heads.getIndex()); @@ -848,13 +852,13 @@ public Collection repack() throws IOException { } if (!nonHeads.isEmpty()) { Pack rest = writePack(nonHeads, allHeadsAndTags, PackWriter.NONE, - tagTargets, excluded); + PackWriter.NONE, tagTargets, excluded); if (rest != null) ret.add(rest); } if (!txnHeads.isEmpty()) { Pack txn = writePack(txnHeads, PackWriter.NONE, PackWriter.NONE, - null, excluded); + PackWriter.NONE, null, excluded); if (txn != null) ret.add(txn); } @@ -1123,6 +1127,7 @@ private Set listNonHEADIndexObjects() private Pack writePack(@NonNull Set want, @NonNull Set have, @NonNull Set tags, + @NonNull Set excludedRefsTips, Set tagTargets, List excludeObjects) throws IOException { checkCancelled(); @@ -1154,7 +1159,8 @@ private Pack writePack(@NonNull Set want, if (excludeObjects != null) for (ObjectIdSet idx : excludeObjects) pw.excludeObjects(idx); - pw.preparePack(pm, want, have, PackWriter.NONE, tags); + pw.preparePack(pm, want, have, PackWriter.NONE, + union(tags, excludedRefsTips)); if (pw.getObjectCount() == 0) return null; checkCancelled(); @@ -1267,6 +1273,15 @@ private Pack writePack(@NonNull Set want, } } + private Set union(Set tags, + Set excludedRefsHeadsTips) { + HashSet unionSet = new HashSet<>( + tags.size() + excludedRefsHeadsTips.size()); + unionSet.addAll(tags); + unionSet.addAll(excludedRefsHeadsTips); + return unionSet; + } + private void checkCancelled() throws CancelledException { if (pm.isCancelled() || Thread.currentThread().isInterrupted()) { throw new CancelledException(JGitText.get().operationCanceled); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 924328d8a..6f76326bc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -620,6 +620,12 @@ public final class ConfigConstants { */ public static final String CONFIG_KEY_BITMAP_EXCESSIVE_BRANCH_COUNT = "bitmapexcessivebranchcount"; + /** + * The "pack.bitmapExcludedRefsPrefixes" key + * @since 5.13.2 + */ + public static final String CONFIG_KEY_BITMAP_EXCLUDED_REFS_PREFIXES = "bitmapexcludedrefsprefixes"; + /** * The "pack.bitmapInactiveBranchAgeInDays" key * @since 5.8 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java index 6aa8be642..a10f6cf88 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java @@ -16,6 +16,7 @@ import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BITMAP_CONTIGUOUS_COMMIT_COUNT; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BITMAP_DISTANT_COMMIT_SPAN; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BITMAP_EXCESSIVE_BRANCH_COUNT; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BITMAP_EXCLUDED_REFS_PREFIXES; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BITMAP_INACTIVE_BRANCH_AGE_INDAYS; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BITMAP_RECENT_COMMIT_COUNT; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BUILD_BITMAPS; @@ -225,6 +226,14 @@ public class PackConfig { */ public static final int DEFAULT_BITMAP_INACTIVE_BRANCH_AGE_IN_DAYS = 90; + /** + * Default refs prefixes excluded from the calculation of pack bitmaps. + * + * @see #setBitmapExcludedRefsPrefixes(String[]) + * @since 5.13.2 + */ + public static final String[] DEFAULT_BITMAP_EXCLUDED_REFS_PREFIXES = new String[0]; + /** * Default max time to spend during the search for reuse phase. This * optimization is disabled by default: {@value} @@ -285,6 +294,8 @@ public class PackConfig { private int bitmapInactiveBranchAgeInDays = DEFAULT_BITMAP_INACTIVE_BRANCH_AGE_IN_DAYS; + private String[] bitmapExcludedRefsPrefixes = DEFAULT_BITMAP_EXCLUDED_REFS_PREFIXES; + private Duration searchForReuseTimeout = DEFAULT_SEARCH_FOR_REUSE_TIMEOUT; private boolean cutDeltaChains; @@ -1144,6 +1155,27 @@ public void setBitmapInactiveBranchAgeInDays(int ageInDays) { bitmapInactiveBranchAgeInDays = ageInDays; } + /** + * Get the refs prefixes excluded from the Bitmap. + * + * @return the refs prefixes excluded from the Bitmap. + * @since 5.13.2 + */ + public String[] getBitmapExcludedRefsPrefixes() { + return bitmapExcludedRefsPrefixes; + } + + /** + * Set the refs prefixes excluded from the Bitmap. + * + * @param excludedRefsPrefixes + * the refs prefixes excluded from the Bitmap. + * @since 5.13.2 + */ + public void setBitmapExcludedRefsPrefixes(String[] excludedRefsPrefixes) { + bitmapExcludedRefsPrefixes = excludedRefsPrefixes; + } + /** * Set the max time to spend during the search for reuse phase. * @@ -1220,6 +1252,12 @@ public void fromConfig(Config rc) { setBitmapInactiveBranchAgeInDays(rc.getInt(CONFIG_PACK_SECTION, CONFIG_KEY_BITMAP_INACTIVE_BRANCH_AGE_INDAYS, getBitmapInactiveBranchAgeInDays())); + String[] excludedRefsPrefixesArray = rc.getStringList(CONFIG_PACK_SECTION, + null, + CONFIG_KEY_BITMAP_EXCLUDED_REFS_PREFIXES); + if(excludedRefsPrefixesArray.length > 0) { + setBitmapExcludedRefsPrefixes(excludedRefsPrefixesArray); + } setSearchForReuseTimeout(Duration.ofSeconds(rc.getTimeUnit( CONFIG_PACK_SECTION, null, CONFIG_KEY_SEARCH_FOR_REUSE_TIMEOUT, From 8040936f8aee4f0637339ab7c1dcfc56fe612cff Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 31 Jan 2023 23:45:07 +0100 Subject: [PATCH 6/8] Silence API warnings introduced by I466dcde6 Change-Id: I510510da34d33757c2f83af8cd1e26f6206a486a --- .../.settings/.api_filters | 17 +++++++ org.eclipse.jgit/.settings/.api_filters | 48 +++++++++++++++++-- 2 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 org.eclipse.jgit.http.server/.settings/.api_filters diff --git a/org.eclipse.jgit.http.server/.settings/.api_filters b/org.eclipse.jgit.http.server/.settings/.api_filters new file mode 100644 index 000000000..951a53bf3 --- /dev/null +++ b/org.eclipse.jgit.http.server/.settings/.api_filters @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 27b322c91..cc4916cf8 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -1,14 +1,12 @@ - - + + - + - - @@ -24,6 +22,38 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -32,6 +62,14 @@ + + + + + + + + From 76508320025a2258ccf169de2f559ad01f27281b Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 4 Oct 2022 15:42:25 +0200 Subject: [PATCH 7/8] FetchCommand: fix fetchSubmodules to work on a Ref to a blob FetchCommand#fetchSubmodules assumed that FETCH_HEAD can always be parsed as a tree. This isn't true if it refers to a Ref referring to a BLOB. This is e.g. used in Gerrit for Refs like refs/sequences/changes which are used to implement sequences stored in git. Change-Id: I414f5b7d9f2184b2d7d53af1dfcd68cccb725ca4 --- org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java index 90c1515b0..7290d83df 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java @@ -140,6 +140,9 @@ private void fetchSubmodules(FetchResult results) if (fetchHead == null) { return; } + if (revWalk.parseAny(fetchHead).getType() == Constants.OBJ_BLOB) { + return; + } walk.setTree(revWalk.parseTree(fetchHead)); while (walk.next()) { try (Repository submoduleRepo = walk.getRepository()) { From 21e902dd7fa4ff53dc35fd7c48f8b5edc52f8eea Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Wed, 18 May 2022 13:31:30 +0100 Subject: [PATCH 8/8] Shortcut during git fetch for avoiding looping through all local refs The FetchProcess needs to verify that all the refs received point to objects that are reachable from the local refs, which could be very expensive but is needed to avoid missing objects exceptions because of broken chains. When the local repository has a lot of refs (e.g. millions) and the client is fetching a non-commit object (e.g. refs/sequences/changes in Gerrit) the reachability check on all local refs can be very expensive compared to the time to fetch the remote ref. Example for a 2M refs repository: - fetching a single non-commit object: 50ms - checking the reachability of local refs: 30s A ref pointing to a non-commit object doesn't have any parent or successor objects, hence would never need to have a reachability check done. Skipping the askForIsComplete() altogether would save the 30s time spent in an unnecessary phase. Signed-off-by: Luca Milanesio Change-Id: I09ac66ded45cede199ba30f9e71cc1055f00941b --- .../eclipse/jgit/transport/FetchProcess.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java index 2cedd4b07..507795af5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java @@ -47,6 +47,7 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.revwalk.ObjectWalk; +import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.util.StringUtils; @@ -378,11 +379,19 @@ private void updateFETCH_HEAD(FetchResult result) throws IOException { private boolean askForIsComplete() throws TransportException { try { try (ObjectWalk ow = new ObjectWalk(transport.local)) { - for (ObjectId want : askFor.keySet()) - ow.markStart(ow.parseAny(want)); - for (Ref ref : localRefs().values()) - ow.markUninteresting(ow.parseAny(ref.getObjectId())); - ow.checkConnectivity(); + boolean hasCommitObject = false; + for (ObjectId want : askFor.keySet()) { + RevObject obj = ow.parseAny(want); + ow.markStart(obj); + hasCommitObject |= obj.getType() == Constants.OBJ_COMMIT; + } + // Checking connectivity makes sense on commits only + if (hasCommitObject) { + for (Ref ref : localRefs().values()) { + ow.markUninteresting(ow.parseAny(ref.getObjectId())); + } + ow.checkConnectivity(); + } } return true; } catch (MissingObjectException e) {