summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Add verification in GcKeepFilesTest that bitmaps are generatedLuca Milanesio2023-07-051-0/+4
| | | | | | | | | | | | | | | | | | | The packfiles with the .keep extensions are meant to prevent a packfile from being processed or removed during GC. From the point of view of the GC process then, the associated packfile should be completely transparent: - it should not included in the repacked file - it should not pruned - its objects should be left untouched, even if unreachable - the GC process, including the bitmap generation should continue as usual, as the the packfiles with .keep file did not exist Add one explicit test for making sure that the management of .keep file is also transparent to the generation of bitmaps, which are still generated if a .keep file exists. Bug: 582039 Change-Id: I14f6adc3f961c606fbc617e51ea6ed6e2ef8604f
* Express the explicit intention of creating bitmaps in GCLuca Milanesio2023-07-052-5/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Add an explicit flag to PackWriter for allowing the GC.repack() phase to explicitly generate bitmaps only for the heads packfile and not for the others. Previously the bitmap generation was conditioned to the presence of object ids exclusion from the PackWriter. The introduction of the bitmap generation in the PackWriter done in Icdb0cdd66 has accidentally made the .keep files not completely transparent, because their presence have disabled the generation of the bitmap index, even if the generation of bitmaps is enabled. This bug has been an accidental consequence of the intention of the bitmap generator to avoid generating bitmaps for the non-heads packfile, however the implementation done by Colby decided to use the excludeInPacks variable (see [1]) which is unfortunately also used for excluding the packfiles having an associated .keep file (see [2]). [1] https://git.eclipse.org/r/c/jgit/jgit/+/7940/18/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java#1617 [2] https://git.eclipse.org/r/plugins/gitiles/jgit/jgit/+/dafcb8f6db82b899c917832768f1c240d273190c/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/GC.java#506 Bug: 582039 Change-Id: Id722e68d9ff4ac24e73bf765ab11017586b6766e
* GC: prune all packfiles after the loosen phaseLuca Milanesio2023-07-051-1/+3
| | | | | | | | | | | | | When loosening the objects inside the packfiles to be pruned, make sure that the packfile list is stable and prune all the files after the loosening is done. This prevents a series of exceptions previously thrown when loosening the packfiles, due to the too early pruning of the packfiles that were still in the pack list. Bug: 581532 Change-Id: I776776e2e083f1fa749d53f965bf50f919823b4f
* Prepare 5.13.3-SNAPSHOT buildsMatthias Sohn2023-06-2290-503/+561
| | | | Change-Id: I02b9388c8bc1c266bb29b4502504d137dd42142f
* JGit v5.13.2.202306221912-rv5.13.2.202306221912-rMatthias Sohn2023-06-2288-121/+121
| | | | | Signed-off-by: Matthias Sohn <matthias.sohn@sap.com> Change-Id: Id0ee779fba85a6d5557f6319969adb2c74feebcf
* Revert "RefDirectory: Throw exception if CAS of packed ref list fails"Martin Fick2023-06-071-16/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 9c33f7364d41956240818ba12d8b79d5ea846162. Reason for revert: This change was based on the false claim that the packedrefs file lock is held while the CAS is being done, but it is actually released before the CAS (the in memory lock is still held, however that does not prevent external actors from updating the packedrefs files and then another thread from subsequently re-reading it and updating the in memory packedRefList). Although reverting this change can cause the CAS to fail, it should not actually matter since the failure would indicate that another thread has already updated the in memory packedRefList to either the same version this thread was trying to update it too, or to a more recent version. Either way, failing the CAS is then appropriate and should not be problematic. Although this change reverts the code in the RefDirectory class, it keeps the "improvements" to the test so that it continues to pass reliably. The reason for the quotes around the word "improvements" is because I believe the test alteration actually dramatically changes the intent of the test, and that the original intent of the test is untestable with the GC and RefDirectory classes as is. Bug: 582044 Change-Id: I3acee7527bb542996dcdfaddfb2bdb45ec444db5 Signed-off-by: Martin Fick <quic_mfick@quicinc.com> (cherry picked from commit c5617711a1b4d5d0807cc7eed702b78d114d46b3)
* GcConcurrentTest: @Ignore flaky testInterruptGcJonathan Tan2023-04-271-0/+2
| | | | | | | | | During my development of Id7721cc5b7ea650e77c2db47042715487983cae6, I have found this test to be flaky when run by CI. As a speculative fix, mark this test as @Ignore so it won't be run. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Change-Id: Idfe04d7f1fb72a772d4c8d249ca86a9c2eec0b1a
* Fix CommitTemplateConfigTestMatthias Sohn2023-04-271-4/+1
| | | | | | | The cherry-picked 61d4e313 doesn't match 5.13 APIs which changed in newer versions. Change-Id: I61ed0242472ed822028d86d3038f956f6bd5735c
* [bazel] Skip ConfigTest#testCommitTemplatePathInHomeDirecoryMatthias Sohn2023-04-263-24/+62
| | | | | | | | | | | | | Move this test to another class and skip it when running tests with bazel since the bazel test runner does not allow to create files in the home directory. FS#userHome retrieves the home directory on the first call and caches it for subsequent calls to avoid overhead in case path translation is required (currently on cygwin). This prevents that the test can mock the home directory using MockSystemReader like SshTestHarness does. Change-Id: I6a22f37f4a19eb4b4935509eae508a23e56db7aa
* Demote severity of some error prone bug patterns to warningsDavid Ostrovsky2023-04-261-3/+3
| | | | | | | | | | | The code is not violation free, so demote the severity of these bug patterns to warning: o DefaultCharset o FutureReturnValueIgnored o UnusedException Change-Id: Ie886a4a247770a74953385f018498ac2515ed209
* Merge branch 'stable-5.12' into stable-5.13Matthias Sohn2023-04-207-6/+102
|\ | | | | | | | | | | | | | | | | | | | | | | * stable-5.12: Add missing since tag for SshBasicTestBase Add missing since tag for SshTestHarness#publicKey2 Silence API errors Prevent infinite loop rescanning the pack list on PackMismatchException Remove blank in maven.config Change-Id: Ibe6652374ab5971105e62b05279f218c8c130fee
| * Merge branch 'stable-5.11' into stable-5.12stable-5.12Matthias Sohn2023-04-207-7/+89
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * stable-5.11: Add missing since tag for SshBasicTestBase Add missing since tag for SshTestHarness#publicKey2 Silence API errors Prevent infinite loop rescanning the pack list on PackMismatchException Remove blank in maven.config Change-Id: I25bb99687b969f9915a7cbda8d1332bec778096a
| | * Add missing since tag for SshBasicTestBasestable-5.11Matthias Sohn2023-04-201-0/+2
| | | | | | | | | | | | Change-Id: Iad8ae9bb526418b279dc54a5e9d0c877c1eca475
| | * Merge branch 'stable-5.10' into stable-5.11Matthias Sohn2023-04-208-75/+87
| | |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * stable-5.10: Add missing since tag for SshTestHarness#publicKey2 Silence API errors Prevent infinite loop rescanning the pack list on PackMismatchException Remove blank in maven.config Migrated "Prevent infinite loop rescanning the pack list on PackMismatchException" to refactoring done in https://git.eclipse.org/r/q/topic:restore-preserved-packs Change-Id: I0fb77bb9b498d48d5da88a93486b99bf8121e3bd
| | | * Add missing since tag for SshTestHarness#publicKey2stable-5.10Matthias Sohn2023-04-202-0/+14
| | | | | | | | | | | | | | | | Change-Id: Ib6e4945340d2e1761dc0e787bdbe72286cdc95bc
| | | * Silence API errorsMatthias Sohn2023-04-201-0/+17
| | | | | | | | | | | | | | | | Change-Id: I367c05c43df3385c33ce76bc10b2dc3bd330665c
| | | * Merge branch 'stable-5.9' into stable-5.10Matthias Sohn2023-04-204-7/+73
| | | |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * stable-5.9: Prevent infinite loop rescanning the pack list on PackMismatchException Remove blank in maven.config Change-Id: I15ff2d7716ecaceb0eb87b8823d85670f5db709d
| | | | * Prevent infinite loop rescanning the pack list on PackMismatchExceptionstable-5.9Matthias Sohn2023-04-193-7/+76
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We found, when analysing an incident where Gerrit's gc runner thread got stuck, that we can end up in an infinite loop in ObjectDirectory#openPackedObject which tries to rescan the pack list and starts over trying to open a packed object in an unconfined loop if it catches a PackMismatchException. Here the relevant part of a thread dump we created while the gc runner was stuck: "WorkQueue-2[java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask@350812a3[Not completed, task = java.util.concurrent.Executors$RunnableAdapter@5425d7ee]]" #72 tid=0x00007f73cee1c800 nid=0x584 runnable [0x00007f7392d57000] java.lang.Thread.State: RUNNABLE at org.eclipse.jgit.internal.storage.file.WindowCache.removeAll(WindowCache.java:716) at org.eclipse.jgit.internal.storage.file.WindowCache.purge(WindowCache.java:399) at org.eclipse.jgit.internal.storage.file.PackFile.close(PackFile.java:296) at org.eclipse.jgit.internal.storage.file.ObjectDirectory.reuseMap(ObjectDirectory.java:973) at org.eclipse.jgit.internal.storage.file.ObjectDirectory.scanPacksImpl(ObjectDirectory.java:904) at org.eclipse.jgit.internal.storage.file.ObjectDirectory.scanPacks(ObjectDirectory.java:895) - locked <0x000000050a498f60> (a java.util.concurrent.atomic.AtomicReference) at org.eclipse.jgit.internal.storage.file.ObjectDirectory.searchPacksAgain(ObjectDirectory.java:794) at org.eclipse.jgit.internal.storage.file.ObjectDirectory.openPackedObject(ObjectDirectory.java:465) at org.eclipse.jgit.internal.storage.file.ObjectDirectory.openPackedFromSelfOrAlternate(ObjectDirectory.java:417) at org.eclipse.jgit.internal.storage.file.ObjectDirectory.openObject(ObjectDirectory.java:408) at org.eclipse.jgit.internal.storage.file.WindowCursor.open(WindowCursor.java:132) at org.eclipse.jgit.lib.ObjectReader$1.open(ObjectReader.java:279) at org.eclipse.jgit.revwalk.RevWalk$2.next(RevWalk.java:1031) at org.eclipse.jgit.internal.storage.pack.PackWriter.findObjectsToPack(PackWriter.java:1911) at org.eclipse.jgit.internal.storage.pack.PackWriter.preparePack(PackWriter.java:960) at org.eclipse.jgit.internal.storage.pack.PackWriter.preparePack(PackWriter.java:876) at org.eclipse.jgit.internal.storage.file.GC.writePack(GC.java:1168) at org.eclipse.jgit.internal.storage.file.GC.repack(GC.java:852) at org.eclipse.jgit.internal.storage.file.GC.doGc(GC.java:269) at org.eclipse.jgit.internal.storage.file.GC.gc(GC.java:220) at org.eclipse.jgit.api.GarbageCollectCommand.call(GarbageCollectCommand.java:179) at com.google.gerrit.server.git.GarbageCollection.run(GarbageCollection.java:112) at com.google.gerrit.server.git.GarbageCollection.run(GarbageCollection.java:75) at com.google.gerrit.server.git.GarbageCollection.run(GarbageCollection.java:71) at com.google.gerrit.server.git.GarbageCollectionRunner.run(GarbageCollectionRunner.java:76) at com.google.gerrit.server.logging.LoggingContextAwareRunnable.run(LoggingContextAwareRunnable.java:103) at java.util.concurrent.Executors$RunnableAdapter.call(java.base@11.0.18/Executors.java:515) at java.util.concurrent.FutureTask.runAndReset(java.base@11.0.18/FutureTask.java:305) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(java.base@11.0.18/ScheduledThreadPoolExecutor.java:305) at com.google.gerrit.server.git.WorkQueue$Task.run(WorkQueue.java:612) at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@11.0.18/ThreadPoolExecutor.java:1128) at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@11.0.18/ThreadPoolExecutor.java:628) at java.lang.Thread.run(java.base@11.0.18/Thread.java:829) The code in ObjectDirectory#openPackedObject [1] apparently assumes that this is caused by a transient problem which it can resume from by retrying. We use `core.trustFolderStat = false` on this server since it uses NFS. The incident we had showed that we can enter into an infinite loop here if there is a permanent mismatch between a pack file and its corresponding pack index. I am not yet sure how this can happen. Break the infinite loop by limiting the number of attempts rescanning the pack list to 5 retries. When we exceed this threshold set the type of the PackMismatchException to permanent and rethrow it which breaks the infinite loop. Also apply the same limit in #getPackedObjectSize and #selectObjectRepresentation where we use similar retry loops. [1] https://git.eclipse.org/r/plugins/gitiles/jgit/jgit/+/011c26ff36b9e76c84fc2459e337f159c0f55a9a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java#465 Change-Id: I20fb63bcc1fdc3a03d39b963f06a90e6f0ba73dc
| | | | * Remove blank in maven.configMatthias Sohn2023-04-191-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Maven 3.9.1 doesn't accept this whitespace. Change-Id: I0f6e3652b1e581615c370d35bc782184712ac922
* | | | | Remove blank in maven.configMatthias Sohn2023-04-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Maven 3.9.1 doesn't accept this whitespace. Change-Id: I0f6e3652b1e581615c370d35bc782184712ac922
* | | | | DirCache: support option index.skipHashMatthias Sohn2023-03-285-12/+137
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Support the new option index.skipHash which was introduced in git 2.40 [1]. If it is set to true skip computing the git index checksum. This accelerates Git commands that manipulate the index, such as git add, git commit, or git status. Instead of storing the checksum, write a trailing set of bytes with value zero, indicating that the computation was skipped. Accept a skipped checksum consisting of 20 null bytes when reading the index since the option could have been set to true at the time when the index was written. [1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-indexskipHash Bug: 581723 Change-Id: I28ebe44c5ca1cbcb882438665d686452a0c111b2
* | | | | GC: Close File.lines streamXing Huang2023-03-231-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | From File#lines javadoc: The returned stream from File Lines encapsulates a Reader. If timely disposal of file system resources is required, the try-with-resources construct should be used to ensure that the stream's close method is invoked after the stream operations are completed. Wrap File.lines with try-with-resources. Change-Id: I82c6faa3ef1083f6c7e964f96e9540b4db18eee8 Signed-off-by: Xing Huang <xingkhuang@google.com> (cherry picked from commit 172a207945da376b6b4143305aef2af56f7c42e2)
* | | | | If tryLock fails to get the lock another gc has itMatthias Sohn2023-02-221-1/+1
| | | | | | | | | | | | | | | | | | | | Change-Id: Ifd3bbcc5e0591883b774d23256949a83010ea134
* | | | | Fix GcConcurrentTest#testInterruptGcMatthias Sohn2023-02-221-5/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With the new GC.PidLock interrupting a running GC throws a ClosedByInterruptException. Change-Id: I7ccea1ae9a43d4edfdab2fcfd1324c64cc22b38f
* | | | | Don't swallow IOException in GC.PidLock#lockMatthias Sohn2023-02-221-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This broke the test GcConcurrentTest#testInterruptGc which expects ClosedByInterruptException when the thread doing gc is interrupted. Change-Id: I89e02fc37aceeccb04c20cfc5b71cb8fa21793d6
* | | | | Check if FileLock is valid before using or releasing itMatthias Sohn2023-02-221-2/+2
| | | | | | | | | | | | | | | | | | | | Change-Id: I23ba67b61b9b03772f33a929c080c0d02b8c8652
* | | | | Acquire file lock "gc.pid" before running gcMatthias Sohn2023-02-213-8/+181
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Git guards gc by locking a lock file "gc.pid" before starting execution. The lock file contains the pid and hostname of the process holding the lock. Git tries to kill the process holding that lock if the lock file wasn't modified in the last 12 hours and was started from the same host. Teach JGit to acquire this lock before running gc but skip execution if another process already holds the lock. Killing the other process could be undesired if it's a long running application. If the lock file wasn't modified in the last 12 hours try to lock it and run gc if locking succeeds. Register a shutdown hook for the lock file to ensure it is cleaned up if the process is gracefully killed. Change-Id: I00b838dcbf4fb0d03863bf7a2cd86b743c6c6971
* | | | | Silence API errors introduced by 9424052fMatthias Sohn2023-02-211-0/+12
| | | | | | | | | | | | | | | | | | | | Change-Id: Ia9e619a8fa06648086b583c994e4b107ae06c44d
* | | | | Add pack options to preserve and prune old pack filesMatthias Sohn2023-02-114-8/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add the options - pack.preserveOldPacks - pack.prunePreserved This allows to configure in git config if old packs should be preserved during gc and pruned during the next gc. The original implementation in 91132bb0 only allows to set these options using the API. Change-Id: I5b23ab4f317d12f5ccd234401419913e8263cc9a
* | | | | Allow to perform PackedBatchRefUpdate without locking loose refsSaša Živkov2023-02-032-3/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add another newBatchUpdate method in the RefDirectory where we can control if the created PackedBatchRefUpdate will lock the loose refs or not. This can be useful in cases when we run programs which have exclusive access to a Git repository and we know that locking loose refs is unnecessary and just a performance loss. Change-Id: I7d0932eb1598a3871a2281b1a049021380234df9 (cherry picked from commit cb90ed08526bd51f04e5d72e3ba3cf5bd30c11e4)
* | | | | Document option "core.sha1Implementation" introduced in 59029aecMatthias Sohn2023-02-021-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | Bug: 580310 Change-Id: I10f3d6f6b5af7ab96683994c9cbd85e6c18a5084
* | | | | Shortcut during git fetch for avoiding looping through all local refsLuca Milanesio2023-02-011-5/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <luca.milanesio@gmail.com> Change-Id: I09ac66ded45cede199ba30f9e71cc1055f00941b
* | | | | FetchCommand: fix fetchSubmodules to work on a Ref to a blobMatthias Sohn2023-01-311-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | | | | Silence API warnings introduced by I466dcde6Matthias Sohn2023-01-312-5/+60
| | | | | | | | | | | | | | | | | | | | Change-Id: I510510da34d33757c2f83af8cd1e26f6206a486a
* | | | | Allow the exclusions of refs prefixes from bitmapLuca Milanesio2023-01-315-4/+82
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <luca.milanesio@gmail.com>
* | | | | PackWriterBitmapPreparer: do not include annotated tags in bitmapLuca Milanesio2023-01-312-0/+37
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <luca.milanesio@gmail.com> Change-Id: Iff2bfc6587153001837220189a120ead9ac649dc
* | | | | BatchingProgressMonitor: avoid int overflow when computing percentageMatthias Sohn2023-01-312-3/+86
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When cloning huge repositories I observed percentage of object counts turning negative. This happened if lastWork * 100 exceeded Integer.MAX_VALUE. Change-Id: Ic5f5cf5a911a91338267aace4daba4b873ab3900
* | | | | Speedup GC listing objects referenced from reflogsMatthias Sohn2023-01-235-41/+36
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | | | | FileSnapshotTest: Add more MISSING_FILE coverageNasser Grainawi2023-01-061-0/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <quic_nasserg@quicinc.com>
* | | | | Silence API warningsMatthias Sohn2022-11-203-0/+96
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | introduced by - addition of configurable SHA1 implementation in 5.13.2 - 3-digit @since 5.9.1 annotations on GitServlet methods Change-Id: If19853fcc5e3677e5b18e8e3fbbcd2773378dffc
* | | | | [benchmarks] Remove profiler configurationMatthias Sohn2022-11-154-8/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Profiler configuration can be added when required. It was commented out in most benchmarks. Change-Id: I725f98757f7d4d2ba2589658e34e2fd6fbbbedee
* | | | | Add SHA1 benchmarkMatthias Sohn2022-11-152-0/+104
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Results on a Mac M1 max: size SHA1Native SHA1Java SHA1Java without with collision collision detection detection [kB] [us/op] [us/op] [us/op] --------------------------------------------- 1 3.662 4.200 4.707 2 7.053 7.868 8.928 4 13.883 15.149 17.608 8 27.225 30.049 35.237 16 54.014 59.655 70.867 32 106.457 118.022 140.403 64 212.712 237.702 281.522 1024 3469.519 3868.883 4637.287 131072 445011.724 501751.992 604061.308 1048576 3581702.104 4008087.854 4831023.563 The last 3 sizes (1, 128, 1024 MB) weren't committed here to limit the total runtime. Bug: 580310 Change-Id: I7d0382fd4aa4c4734806b12e96b671bee37d26e3
* | | | | [benchmarks] Set version of maven-compiler-plugin to 3.8.1Matthias Sohn2022-11-151-0/+1
| | | | | | | | | | | | | | | | | | | | Change-Id: Ib14db133c76a55358ea79663ef38d9fb47a67f45
* | | | | Fix running JMH benchmarksMatthias Sohn2022-11-151-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Without this I sometimes hit the error: $ java -jar target/benchmarks.jar Exception in thread "main" java.lang.RuntimeException: ERROR: Unable to find the resource: /META-INF/BenchmarkList at org.openjdk.jmh.runner.AbstractResourceReader.getReaders(AbstractResourceReader.java:98) at org.openjdk.jmh.runner.BenchmarkList.find(BenchmarkList.java:124) at org.openjdk.jmh.runner.Runner.internalRun(Runner.java:253) at org.openjdk.jmh.runner.Runner.run(Runner.java:209) at org.openjdk.jmh.Main.main(Main.java:71) Change-Id: Iea9431d5f332f799d55a8a2d178c79a2ef0da22b
* | | | | Add option to allow using JDK's SHA1 implementationMatthias Sohn2022-11-156-493/+823
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The change If6da9833 moved the computation of SHA1 from the JVM's JCE to a pure Java implementation with collision detection. The extra security for public sites comes with a cost of slower SHA1 processing compared to the native implementation in the JDK. When JGit is used internally and not exposed to any traffic from external or untrusted users, the extra cost of the pure Java SHA1 implementation can be avoided, falling back to the previous native MessageDigest implementation. Bug: 580310 Change-Id: Ic24c0ba1cb0fb6282b8ca3025ffbffa84035565e
* | | | | Ignore IllegalStateException if JVM is already shutting downMatthias Sohn2022-10-271-8/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Trying to register/unregister a shutdown hook when the JVM is already in shutdown throws an IllegalStateException. Ignore this exception since we can't do anything about it. Bug: 580953 Change-Id: I8fc6fdd5585837c81ad0ebd6944430856556d90e
* | | | | UploadPack: don't prematurely terminate timer in case of errorMatthias Sohn2022-06-302-8/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In uploadWithExceptionPropagation don't prematurely terminate timer in case of error to enable reporting it to the client. Expose a close method so that callers can terminate it at the appropriate time. If the timer is already terminated when trying to report it to the client this failed with the error java.lang.IllegalStateException: "Timer already terminated". Bug: 579670 Change-Id: I95827442ccb0f9b1ede83630cf7c51cf619c399a
* | | | | Merge "Do not create reflog for remote tracking branches during clone" into ↵Matthias Sohn2022-06-263-2/+71
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | stable-5.13
| * | | | | Do not create reflog for remote tracking branches during cloneLuca Milanesio2022-06-253-2/+71
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using JGit on a non-bare repository, the CloneCommand it previously created local reflogs for all branches including remote tracking ones, causing the generation of a potentially large number of files on the local filesystem. The creation of the remote-tracking branches (refs/remotes/*) during clone is not an issue for the local filesystem because all of them are stored in a single packed-refs file. However, the creation of a large number of ref logs on a local filesystem IS an issue because it may not be tuned or initialised in term of inodes to contain a very large number of files. When a user (or a CI system) performs the CloneCommand against a potentially large repository (e.g., millions of branches), it is interested in working or validating a single branch or tag and is unlikely to work with all the remote-tracking branches. The eager creation of a reflogs for all the remote-tracking branches is not just a performance issue but may also compromise the ability to use JGit for cloning a large repository. The behaviour implemented in this change is also consistent with the optimisation done in the C code-base [1]. We differentiate between clone and fetch commands using --branch <initialBranch> option, that is only available in clone command, and is set as HEAD per default. [1] https://github.com/git/git/commit/58f233ce1ed67bbc31a429fde5c65d5050fdbd7d Bug: 579805 Change-Id: I58d0d36a8a4ce42e0f59b8bf063747c4b81bd859 Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
* | | | | | UploadPack: do not check reachability of visible SHA1sLuca Milanesio2022-06-251-1/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When JGit needs to serve a Git client requesting SHA1s during the want phase, it needs to make a full reachability check from the advertised refs to the ones requested to keep all objects in the correct scope of confidentiality allowed by the avertised refs. The check is also performed when the SHA1 corresponds to one of the tips of the advertised refs which is a waste of resources. Example: fetch> ref-prefix refs/heads/foo fetch< 900505eb8ce8ced2a1757906da1b25c357b9654e refs/heads/foo fetch< 0000 fetch> command=fetch fetch> 0001 fetch> thin-pack fetch> ofs-delta fetch> want 900505eb8ce8ced2a1757906da1b25c357b9654e The SHA1 in the want is the tip of refs/heads/foo and therefore the full reachability check can be shortened and resolved more quickly. Change-Id: I49bd9e2464e0bd3bca2abf14c6e9df550d07383b Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-24 03:27:41 +0000