By first checking for null-ness and then for the number of strings to
compare we can get rid of a redundant null check.
Change-Id: I0d9a088352c6c1ffea12bc2cded2c63e5293a8a7
Currently for file-based repositories JGit will go over all refs in the
repository forach `ref-prefix` listed in the `ls-refs` command in git
protocol v2 request.
Native git, uses a different approach, where all refs are read once and
then for each ref, all `ref-prefix` filter values are checked in one
pass.
This change implements this approach in JGit only in the `RefDirectory`
backend. And makes `ref-prefix` filtering ~40% faster for repositories
with packed refs.
Different implementations were tested on a synthetic file repository
with 10k refs in `refs/heads/` and `290k` in `refs/changes`. Before
testing `git pack-refs` command was executed. All results are in
seconds.
Current Impl: 39.340 37.093 35.996
Nested for loops: 25.077 24.742 24.748
Nested streams: 24.827 24.890 27.525
Parallel stream + stream: 23.357 23.318 23.174
Nested parallel streams: 23.490 23.318 23.317
Stream + for loop: 23.147 23.210 23.126
Parallel stream + for loop: 23.317 23.423 22.847
The elapsed time was measured around `getRefByPrefix` call in
`Uploadapack.getFilteredRefs(Collection<String>)` (around lines 952 and
954). For testing a modified version of
`UploadPackTest.testV2LsRefsRefPrefix()` was used. The modifications
here included:
* shadowing protected `repo` variable with `FileRepository` pointing
to the synthetic repo with 300k refs described above,
* mimicking the git client clone request by adding `ref-prefix HEAD`,
`ref-prefix refs/heads/` and `ref-prefix refs/tags/`
Based on the above results, the implementation with parallel stream and
stream was selected.
Bug: 578550
Signed-off-by: Dariusz Luksza <dariusz.luksza@gmail.com>
Change-Id: I6416846c074b611ff6ec9d351dbafcfbcaf68e66
* changes:
Use try-with-resource to ensure UploadPack is closed
Fix hiding field warning
Fix warning for empty code blocks
Fix boxing warnings
errorprone: remove unnecessary parentheses
Update mockito to 5.7.0 and bytebuddy to 1.14.9
Enable Maven reproducible builds
Upgrade bazlets to the latest revision
- configure Maven to run build reproducibly [1]
- use UTC timestamp of checked out commit as build timestamp
- add git-describe, git-commit-id, git-commit-id, git-tags,
git-remote-origin-url to MANIFEST.MF files
- configure cyclonedx-maven-plugin to also use UTC timestamp of
checked out commit
- for packaging build use tycho-buildtimestamp-jgit [2] to ensure
version uses the timestamp of the last commit
- SBOMs are not reproducible by design [3] they should have a build
timestamp matching the time when the build was executed and a serial
number which is a unique UUID per build run. Hence exclude them from
comparison [4].
- Use gmavenplus-plugin to format build timestamps. Maven expects
build timestamp in ISO-8601 format, to replace the qualifier in
versions the timestamp format must be compatible with rules for OSGi
version numbers. Didn't find a way to read the properties set by the
git-commit-id-maven-plugin from another plugin. Hence use JGit in a
groovy script to get the commit time of the current HEAD and provide
it in these two formats.
TODO: packaging build (features and p2 repository) is not yet binary
reproducible since that's not yet supported by Tycho [5], artefacts have
reproducible version numbers but file lastModified timestamps are not
yet reproducible.
Test plan for Maven build:
- build using
mvn clean install"
- verify second build is reproducible:
mvn -T1 clean verify artifact:compare
verification seems not to be thread-safe, hence run it with a single
thread using option -T1
For packaging build (still fails due to non-reproducible file
timestamps):
- build using
mvn -f org.eclipse.jgit.packaging/pom.xml clean install
- verify second build is reproducible:
mvn -T1 -f org.eclipse.jgit.packaging/pom.xml clean verify artifact:compare
[1] https://maven.apache.org/guides/mini/guide-reproducible-builds.html
[2] https://wiki.eclipse.org/Tycho/Reproducible_Version_Qualifiers
[3] https://github.com/CycloneDX/cyclonedx-maven-plugin/issues/84
[4] https://maven.apache.org/plugins/maven-artifact-plugin/compare-mojo.html
[5] https://github.com/eclipse-tycho/tycho/issues/233
Change-Id: I0202f55a1b6ae0edd922cfef638beb39d2ce9417
This reverts commit 3937300f3e.
Reason for revert: This kills performance on the DFS side, that relies on loading the minimal amount of refs and reftables for quick prefix searches.
Reverting as a safe option to keep master in good performance until we decide how to reintroduce this change.
Change-Id: I7b1a3f900d9c78ce95cf0972abb50b6becfe3bb1
Bloom filter computation can be an expensive process and right now it
is invisible to the user.
Report progress while calculating bloom filters.
Log of GC with bloom filter enabled:
Computing commit-graph path bloom filters: 100% (9551/9551)
Computing commit-graph generation numbers: 100% (9551/9551)
Writing out commit-graph: 100% (9551/9551)
Change-Id: Ife65e63ac2c37d064d5f049a366cbb52c3ef6798
The same progress monitor is passed around as parameter and inside the
output stream. The functions use one to start tasks and another to
report progress, which is confusing. The stream needs the monitor to
check cancellations so we cannot remove it from there.
Make all code take the monitor from the stream.
Change-Id: Id3cb9c1cb0bd47318b46ef934a9d4037341e25a7
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
Currenty JGit will go over all refs in the repository for each
`ref-prefix`. This means that refs will be read multiple
times, which leads to subpar performance.
Native git, uses a different approach, where all refs are read once
and then for each ref, all `ref-prefix` filter values are checked in
one pass.
This change implements this approach in JGit. And makes `ref-prefix`
filtering ~28% faster for a repository with fully packed refs
and ~5% when RefTable is used instead of refdir.
Different implementations were tested on a synthetic file repository
with 300k refs. Different implementations were tested for unpacked and
fully packed refs (results are in seconds).
Unpacked refs:
Current Impl: 54.838 57.234 56.138
Nested for loops: 36.094 37.025 36.502
Nested stream's: 36.154 35.989 37.262
Parallel stream + stream: 36.923 37.272 35.362
Nested parallel stream's: 35.512 38.395 36.745
Stream + for loop: 34.950 36.164 37.191
Parallel stream + for loop: 37.695 35.511 35.378
Packed refs:
Current Impl: 39.713 39.954 38.653
Nested for loops: 29.891 29.753 29.377
Nested stream's: 30.340 29.637 30.412
Parallel stream + stream: 28.653 28.254 29.138
Nested parallel stream's: 29.942 28.850 31.030
Stream + for loop: 29.405 29.576 30.539
Parallel stream + for loop: 29.012 29.215 29.380
RefTable:
Current Impl: 0.273 0.294 0.330
Nested for loops: 0.252 0.169 0.215
Nested stream's: 0.252 0.228 0.213
Parallel stream + stream: 0.233 0.259 0.247
Nested parallel stream's: 0.416 0.309 0.340
Stream + for loop: 0.224 0.247 0.242
Parallel stream + for loop: 0.347 0.246 0.346
The elapsed time was measured around `getRefsByPrefix` call in
`UploadPack.getFilteredRefs(Collection<String>)` (around lines 952 and
954).
Based on the above results, the implementation with parallel stream and
stream was selected.
Bug: 578550
Change-Id: Iac3a3aacf897b87b3448c1d528cdac64ad312199
Signed-off-by: Dariusz Luksza <dariusz.luksza@gmail.com>
Parent change introduced using "wanted-refs" as advertised set during
fetchV2, but it tested only a request without wantIds (only
wanted-refs).
Add a second where the request has wanted-refs AND wantId (which
disables the optimization). Change the test to measure the amount of
refs considered advertised, instead of relying in calls to the
refdb.
Change-Id: Id64ec933fd737bae1bfd429c7b8cc05b51a83870
Protocol v2 introduced refs-in-wants and ls-remote with
prefixes. UploadPack already uses prefixes provided by the client
during a v2 ref advertisement (ls-refs). However, when the client
consequently sends another request to fetch a previously advertised
ref (with want-ref lines), the server uses the whole set of advertised
refs to compute reachability.
In repos with many refs, this slows down the reachability checks
setting up and walking through unnecessary refs. For gerrit it can
also break valid requests because in gerrit "all" means "recent" and
the wanted-ref could fall out of the "recent" range when reloading all
refs at fetch time.
Treat wanted-refs like a ref-prefix when calculating the advertised
refs on v2 fetch command. Less refs means a faster setup and less walk
for the reachability checks. Note that wanted-refs filters only over
the refs visible to the user, so this doesn't give any extra
visibility to the caller.
If the request contains also "want <oid>" lines, we cannot use this
optimization. Those objects could be reachable from any visible
branch, not necessarily in the wanted-refs.
Google-Bug: b/122888978
Change-Id: I2a4ae171d4fc5d4cb30b020cb073ad23dd5a66c4
With the useNegotiationTip flag (introduced in change 738dacb), the client sends to the server only the tips of the wanted refs for the negotiation. Some wanted refs may not exist in the client (yet) and our implementation ignores them. So when only non-existing refs are wanted, jgit doesn't send any tips and the server understands it is a full clone.
In useNegotiationTip, send ALL_REFS if any of the wanted refs does not exists locally.
Change-Id: Ide04c5df785b9212abcd9d3cba194515e0af166f
Other getters (e.g. bitmap or commit graph) cover the case that the
pack doesn't have the corresponding extension.
Do the same here to detect this early and avoid an IOException in
openFile.
Change-Id: I29726b7ede0f795d35543453a3e7f92cee872a78
When the exception is thrown, we don't know if it is because the
stream didn't have data or had a wrong header.
Log the read bytes to differentiate these cases.
Change-Id: Ie7612eab39016f5ad7f1bfb2e07cab972dab796f
This was added in
- f103a1d5c6 "Add support for git config repack.packKeptObjects"
- f5f4bf0ad9 "Do not exclude objects in locked packs from bitmap
processing"
Change-Id: Id6af9fe549535c4e92de9080a41ef9f72a6646dd
On Windows reading and replacing a file via renaming concurrently may
fail either in the reader or in the thread renaming the file. For
renaming, FileUtils.rename() has a last-case fallback in which it
deletes the target file before attempting the rename. If a reader reads
at that moment, it will produce an empty config, and the snapshot and
hash may be wrong because the concurrently running save() may set them.
It's not really possible to do all this in a thread-safe manner without
some synchronization. Add a read-write lock to synchronize readers and
writers to avoid at least that JGit steps on its own feet.
Bug: 451508
Change-Id: I7e5f0f26e02f34ba02dc925a445044d3e21389b4
Signed-off-by: Thomas Wolf <twolf@apache.org>
When the atomic move fails on Windows, it may be because some other
thread is currently reading the destination. If we delete the file
then, that reader may get an exception, and conclude the file didn't
exist, even though the rename() would re-create it right away.
Try to avoid this from happening frequently by only deleting the
destination on the last retry. Also don't sleep after the last attempt.
Bug: 451508
Change-Id: I95bb4ec59d6e7efb4a7fc8d67f5df301f690257a
Signed-off-by: Thomas Wolf <twolf@apache.org>
When no config file exists, use FileSnapshot.MISSING_FILE.
Bug: 451508
Change-Id: I8a09cb756a8a4746189da5b3514dfcf81d10b3b1
Signed-off-by: Thomas Wolf <twolf@apache.org>
When multiple branches were to be removed, the git config was updated
after each and every branch. Newly do so only once at the end, after all
branches have been deleted.
Because there may be an exception after some branches have already been
deleted, take care to update the config even if an exception is thrown.
Bug: 451508
Change-Id: I645be8a1a59a1476d421e46933c3f7cbd0639fec
Signed-off-by: Thomas Wolf <twolf@apache.org>
Add a variant of unsetSection() that returns whether it did indeed
change the config. This can be used in to skip saving the config if
it was not changed.
Also fix the iteration over the entries: lastWasMatch was never reset,
and thus all empty lines after a match would be removed.
Change-Id: Iea9e84aa74b1e4bb3c89efe3936fa3a8a09532e5
Signed-off-by: Thomas Wolf <twolf@apache.org>
Ensure that also the fast-forward cases return status
STASH_APPLY_CONFLICTS when applying the stash produces conflicts.
Bug: 582526
Change-Id: Ib989ff431dca6e301eb05156ca054a7115fa6ad5
Signed-off-by: Thomas Wolf <twolf@apache.org>
* stable-6.7:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: I7e0856a5d70d5d155cf6874383ea1f5622d5238a
consuming it directly from Maven Central.
The bundle net.i2p.crypto.eddsa 0.3.0 contains bad OSGi metadata,
earlier it was repackaged in Orbit tweaking its mandatory dependency to
sun.security.x509 to an optional dependency.
This project seems to be orphaned, probably because Java 15 added
support for eddsa with JEP339 [1].
This repackaged bundle is no longer available after Orbit was renovated
[2] to consume the vast majority of bundles directly from Maven Central
without repacking them. Hence we have to workaround this (probably
false) mandatory dependency. For that export an empty dummy package
"sun.security.x509" to satisfy OSGi.
[1] https://openjdk.org/jeps/339
[2] https://github.com/eclipse-orbit/orbit-simrel/issues/15
Change-Id: I2267e15823ebce6cf1d448e1e16a129f703e0f80
* stable-6.6:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: I29241619e6c09933bb856e486f379be10dd609c2
* stable-6.5:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: I7272a22451c0de6b4770767e7bb4e24c81518c20
* stable-6.4:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: I2951d01f5f4581bee20079508cd8ee6ca8554f1f
* stable-6.3:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: I4b94a2b79941c085fa2f62246e8e879aaa85cd3f
* stable-6.2:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: I22b89bf00dcef26b2096d25397aa9a57a745a92b
* stable-6.1:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: Ib4e4fe407dce334c7537bf278baa39db93aa2f09
* stable-6.0:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: I0c9c0b3c206cac03a93b30eda348177a4de35c36
* stable-5.13:
PackConfig: fix @since tags
Remove unused API problem filters
Add support for git config repack.packKeptObjects
Do not exclude objects in locked packs from bitmap processing
Change-Id: Ifeaa4b4f0c5944d4ecd3042be429833ff72b43ed
Change Ide3445e652 introduced the `--pack-kept-objects` option to GC for
including the objects contained in the locked packfiles during the
repack phase.
Whilst this allowed to explicitly pass a command line argument to the
jgit gc program, it did not allow the option to be read from
configuration.
Allow the pack kept objects option to be configured exactly as C-Git
documents [1], by introducing a new `repack.packKeptObjects`
configuration.
`repack.packKeptObjects` defaults to `true`, when the
`pack.buildBitmaps` is `true` (which is the default case), `false`
otherwise.
[1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-repackpackKeptObjects
Bug: 582292
Change-Id: Ia931667277410d71bc079d27c097a57094299840
Packfiles having an equivalent .keep file are associated with in-flight
pushes that haven't been completed, with potentially a set of git
objects not yet referenced by a ref.
If the Git client is not up-to-date, it may result in pushing a
packfile, generating a <packfile>.keep on the server, which
may also contain existing commits due to the lack of Git protocol
negotiation in the git-receive-pack.
The Git protocol negotiation is the phase where the client and the
server exchange the list of refs they have for trying to find a common
base and minimise the amount of objects to be transferred.
The repack phase in GC was previously skipping all objects that were
contained in all packfiles having a <packfile>.keep file associated
(aka "locked packfiles"), which did not take into consideration the
fact that excluding the existing commits would have resulted in the
generation of an invalid bitmap file.
The code for excluding the objects in the locked packfiles was written
well before the bitmap was introduced, hence could not consider a use
case that did not exist at that time.
However, when the bitmap was introduced, the exclusion of locked
packfiles was not changed, hence creating a potential problem.
The issue went unnoticed for many years because the bitmap generation
was disabled when JGit noticed any locked packfiles; however, the
bitmaps are enabled again since Id722e68d9f , and the the issue is now
visible and is impacting the GC repack phase.
Introduce the '--pack-kept-objects' option in GC for including the
objects contained in the locked packfiles during the repack phase,
which is not an issue because of the following:
- If there are any existing commits duplicated in the packfiles
they will be just considered once anyway because the repack doesn't
generate duplicates in the output packfile.
- If there are any new commits that do not have any ref pointing to
them, they will be automatically excluded from the output repacked
packfile.
The same identical solution is adopted in the C implementation of git
in repack.c.
Because the locked packfile is not pruned, any new commits not pointed
by any refs will remain in the repository and there will not be any
accidental pruning or object loss as it is today before this change.
As a side-effect of this change, it is now potentially possible to still
have duplicate BLOBs after GC when the keep packfile contained existing
objects. However, it is way better to keep the duplication until the
next GC phase rather than omitting existing objects from repacking and,
therefore generating an invalid bitmap and incorrect packfile.
Bug: 582292
Bug: 582455
Change-Id: Ide3445e652fcf256a7912f881cb898897c99b8f8
Nasser Grainawi