From cd3fc7a2995c06cf2425f51758094e039c938559 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 18 Jan 2023 17:39:19 +0100 Subject: 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 --- .../src/org/eclipse/jgit/lib/Repository.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'org.eclipse.jgit/src/org/eclipse/jgit/lib') 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 1e8a6c9175..d3b3c6e8a2 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 abstract class Repository implements AutoCloseable { 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 -- cgit v1.2.3 From 611412a05528ba5898cf948f4d3959f1fbc4170a Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 16 Jan 2023 21:58:56 +0100 Subject: 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 --- .../eclipse/jgit/lib/TextProgressMonitorTest.java | 83 ++++++++++++++++++++++ .../eclipse/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 (limited to 'org.eclipse.jgit/src/org/eclipse/jgit/lib') 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 0000000000..55ca2cdea3 --- /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 2caefa4d97..49e295aed8 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 @@ public abstract class BatchingProgressMonitor implements ProgressMonitor { } } 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 @@ public abstract class BatchingProgressMonitor implements ProgressMonitor { 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) -- cgit v1.2.3 From ad977f157242d4d6b5ea4c45b2aa0c15d20b58ae Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Tue, 20 Dec 2022 21:50:19 +0000 Subject: 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 ++++++++++ .../org/eclipse/jgit/internal/storage/file/GC.java | 23 ++++++++++--- .../src/org/eclipse/jgit/lib/ConfigConstants.java | 6 ++++ .../org/eclipse/jgit/storage/pack/PackConfig.java | 38 ++++++++++++++++++++++ 5 files changed, 82 insertions(+), 4 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse/jgit/lib') diff --git a/Documentation/config-options.md b/Documentation/config-options.md index 19bcc33523..b4a0c1d98c 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 55dfa697bc..190ac8b640 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 class GcCommitSelectionTest extends GcTestCase { 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 a14bb411ff..9e97659499 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 class GC { 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 class GC { 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 class GC { } 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 @@ public class GC { 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 @@ public class GC { 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 @@ public class GC { } } + 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 924328d8a6..6f76326bc9 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 6aa8be642d..a10f6cf88a 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_BIGFILE_THRESHOLD; 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 class PackConfig { 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 class PackConfig { 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, -- cgit v1.2.3