Mike Williams [Wed, 21 Oct 2015 20:09:58 +0000 (16:09 -0400)]
Insert duplicate objects to prevent race during garbage collection.
Prior to this change, DfsInserter would not insert an object into a pack
if it already existed in another pack in the repository, even if that
pack was unreachable. Consider this sequence of events:
- Object FOO is pushed to a repository.
- Subsequent ref changes make FOO UNREACHABLE_GARBAGE.
- FOO is subsequently re-inserted using a DfsInserter, but skipped
due to existing in UNREACHABLE_GARBAGE.
- The repository is repacked; FOO will not be written into a new pack
because it is not yet reachable from a reference. If the
UNREACHABLE_GARBAGE packs are deleted, FOO disappears.
- A reference is updated to reference FOO. This reference is now broken
as FOO was removed when the repacking process deleted the
UNREACHABLE_GARBAGE pack that stored the only copy of FOO.
The garbage collector can't safely delete the UNREACHABLE_GARBAGE
pack because FOO might be in the middle of being re-inserted/re-packed.
This change writes a duplicate copy of an object if it only exists in
UNREACHABLE_GARBAGE. This "freshens" the object to give it a chance to
survive long enough to be made reachable through a reference.
Change-Id: I20f2062230f3af3bccd6f21d3b7342f1152a5532 Signed-off-by: Mike Williams <miwilliams@google.com>
Terry Parker [Sat, 31 Oct 2015 00:47:06 +0000 (17:47 -0700)]
[performance] Speed up delta packing
When packing is able to reuse lots of deltas from existing packs, those
objects are marked as "doNotAttemptDelta" and do not contribute to
DeltaTask's computeTopPaths() "totalWeight" calculation.
In the extreme case when all packs are reusable, "totalWeight" will be
zero. DeltaTask.partitionTasks() uses "totalWeight" to determine a
"weightPerThread" size it uses to set up DeltaTasks. When "totalWeight"
is small, partitionTasks() ends up creating a DeltaTask for every
unique path.
For a large repository, the small "weightPerThread" can result in the
creation of >100k tasks (for the MSM 3.10 Linux repository, the count
was ~150k). This makes the "task stealing" mechanism in DeltaTask very
inefficient, because every attempt to steal work does a linear walk
through all tasks, searching for the one with the most work remaining,
which is O(N^2) comparisons. For the MSM 3.10 repository when all
deltas were reusable, PackWriter.parallelDeltaSearch() took
(1615+1633+1458)/3 = 1568 seconds.
The error is that DeltaTask treats the weights of objects marked as
"doNotAttemptDelta" inconsistently. It ignores the weights when
calculating "totalWeight" but uses them when partitioning the tasks.
The fix is to also ignore them when partitioning the tasks.
With this patch applied, PackWriter.parallelDeltaSearch() on the
MSM 3.10 repository when all deltas are reused went from taking
1568 seconds to 62ms (>25k speedup).
This patch also fixes a totalWeight initialization error in
DeltaTask.computeTopPaths().
Change-Id: I2ae37efa83bca42b0e716266ae6aa9d182e76d9c Signed-off-by: Terry Parker <tparker@google.com>
Jonathan Nieder [Tue, 3 Nov 2015 19:42:53 +0000 (11:42 -0800)]
Decrease indentation in setupTipCommitBitmaps
Avoid leaving the reader in suspense by handling the unusual
(!RevCommit) case first. As a nice side effect, there is less nesting
to keep track of in the rest of the loop body.
When the file <git-dir>/hooks/pre-push exists make sure that is is
executing during a push. The pre-push hook runs during git push, after
the remote refs have been updated but before any objects have been
transferred.
Enhance FS.runProcess() to support stdin-redirection and binary data
In order to support filters in gitattributes FS.runProcess() is made
public. Support for stdin redirection has been added. Support for binary
data on stdin/stdout (as used be clean/smudge filters) has been added.
Change-Id: Ice2c152e9391368dc5748d7b825a838e3eb755f9 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Andrey Loskutov [Sun, 1 Nov 2015 12:40:02 +0000 (13:40 +0100)]
Cleaned up various readPipe() threading issues
Fixed random errors in discoverGitSystemConfig() on Linux where the
process error stream was closed by readPipe() before or while
GobblerThread was reading from it.
Marked readPipe() as @Nullable and fixed potential NPE in
discoverGitSystemConfig() on readPipe() return value.
Fixed process error output randomly mixed with other threads log
messages.
Terry Parker [Thu, 29 Oct 2015 04:59:10 +0000 (21:59 -0700)]
Bitmap generation: Add a test of ordering commits by "chains"
When commits are selected for bitmap generation, they are reordered
so that related "chains" of commits are grouped together. Chains are
"subbranches" of commits that may branch off of and re-merge with the
main line. Grouping by chains means that the XOR difference between
consecutive selected commits will be smaller, resulting in better
run-length compression of the XORed bitmaps.
Add a new testSelectionOrderingWithChains() test in a new
GcCommitSelectionTest test class. Also move related GC commit selection
tests out of GcBasicPackingTest and into GcCommitSelectionTest.
Change-Id: I8e80cac29c4ca8193b41c9898e5436c22a659f11 Signed-off-by: Terry Parker <tparker@google.com>
Andrey Loskutov [Wed, 28 Oct 2015 20:17:43 +0000 (21:17 +0100)]
[performance] Remove synthetic access$ methods in dfs, diff and merge
Java compiler must generate synthetic access methods for private methods
and fields of the enclosing class if they are accessed from inner
classes and vice versa.
While invisible in the code, those synthetic access methods exist in the
bytecode and seem to produce some extra execution overhead at runtime
(compared with the direct access to this fields or methods), see
https://git.eclipse.org/r/58948/.
By removing the "private" access modifier from affected methods and
fields we help compiler to avoid generation of synthetic access methods
and hope to improve execution performance.
To validate changes, one can either use javap or use Bytecode Outline
plugin in Eclipse. In both cases one should look for "synthetic
access$<number>" methods at the end of the class and inner class files
in question - there should be none.
NB: don't mix this "synthetic access$" methods up with "public synthetic
bridge" methods generated to allow generic method override return types.
Andrey Loskutov [Wed, 28 Oct 2015 19:52:43 +0000 (20:52 +0100)]
[performance] Remove synthetic access$ methods in lib, util and dircache
Java compiler must generate synthetic access methods for private methods
and fields of the enclosing class if they are accessed from inner
classes and vice versa.
While invisible in the code, those synthetic access methods exist in the
bytecode and seem to produce some extra execution overhead at runtime
(compared with the direct access to this fields or methods), see
https://git.eclipse.org/r/58948/.
By removing the "private" access modifier from affected methods and
fields we help compiler to avoid generation of synthetic access methods
and hope to improve execution performance.
To validate changes, one can either use javap or use Bytecode Outline
plugin in Eclipse. In both cases one should look for "synthetic
access$<number>" methods at the end of the class and inner class files
in question - there should be none.
NB: don't mix this "synthetic access$" methods up with "public synthetic
bridge" methods generated to allow generic method override return types.
Andrey Loskutov [Wed, 28 Oct 2015 19:24:12 +0000 (20:24 +0100)]
[performance] Remove synthetic access$ methods in transport package
Java compiler must generate synthetic access methods for private methods
and fields of the enclosing class if they are accessed from inner
classes and vice versa.
While invisible in the code, those synthetic access methods exist in the
bytecode and seem to produce some extra execution overhead at runtime
(compared with the direct access to this fields or methods), see
https://git.eclipse.org/r/58948/.
By removing the "private" access modifier from affected methods and
fields we help compiler to avoid generation of synthetic access methods
and hope to improve execution performance.
To validate changes, one can either use javap or use Bytecode Outline
plugin in Eclipse. In both cases one should look for "synthetic
access$<number>" methods at the end of the class and inner class files
in question - there should be none.
NB: don't mix this "synthetic access$" methods up with "public synthetic
bridge" methods generated to allow generic method override return types.
Andrey Loskutov [Tue, 27 Oct 2015 22:51:21 +0000 (23:51 +0100)]
[performance] Remove synthetic access$ methods in pack and file packages
Java compiler must generate synthetic access methods for private methods
and fields of the enclosing class if they are accessed from inner
classes and vice versa.
While invisible in the code, those synthetic access methods exist in the
bytecode and seem to produce some extra execution overhead at runtime
(compared with the direct access to this fields or methods), see
https://git.eclipse.org/r/58948/.
By removing the "private" access modifier from affected methods and
fields we help compiler to avoid generation of synthetic access methods
and hope to improve execution performance.
To validate changes, one can either use javap or use Bytecode Outline
plugin in Eclipse. In both cases one should look for "synthetic
access$<number>" methods at the end of the class and inner class files
in question - there should be none.
NB: don't mix this "synthetic access$" methods up with "public synthetic
bridge" methods generated to allow generic method override return types.
Terry Parker [Mon, 19 Oct 2015 22:59:47 +0000 (15:59 -0700)]
Expose bitmap selection parameters via PackConfig
Expose the following bitmap selection parameters via PackConfig:
"bitmapContiguousCommitCount", "bitmapRecentCommitCount",
"bitmapRecentCommitSpan", "bitmapDistantCommitSpan",
"bitmapExcessiveBranchCount", and "bitmapInactiveBranchAge".
The value of bitmapContiguousCommitCount, whereby bitmaps are
created for the most recent N commits in a branch, has never
been verified. If experiments show that they are not valuable,
then we can simplify the implementation so that there is only
a concept of recent and distant commit history (defined by
"bitmapRecentCommitCount"), and the only controls we need are
"bitmapRecentCommitSpan" and "bitmapDistantCommitSpan".
Change-Id: I288bf3f97d6fbfdfcd5dde2699eff433a7307fb9 Signed-off-by: Terry Parker <tparker@google.com>
Terry Parker [Tue, 20 Oct 2015 22:29:38 +0000 (15:29 -0700)]
Update bitmap selection throttling to fully span active branches.
Replace the “bitmapCommitRange” parameter that was recently introduced
with two new parameters: “bitmapExcessiveBranchCount” and
“bitmapInactiveBranchAgeInDays”. If the count of branches does not
exceed “bitmapExcessiveBranchCount”, then the current algorithm is kept
for all branches.
If the branch count is excessive, then the commit time for the tip
commit for each branch is used to determine if a branch is “inactive”.
"Active" branches get full commit selection using the existing
algorithm. "Inactive" branches get fewer bitmaps near the branch tips.
Introduce a "contiguousCommitCount" parameter that always enforces that
the N most recent commits in a branch are selected for bitmaps. The
previous nextSelectionDistance() algorithm created anywhere from 1-100
contiguous bitmaps at branch tips.
For example, consider a branch with commits numbering 0-300, with 0
being the most recent commit. If the most recent 200 commits are not
merge commits and the 200th commit was the last one selected,
nextSelectionDistance() returned 100, causing commits 200-101 to be
ignored. Then a window of size 100 was evaluated, searching for merge
commits. Since no merge commits are found, the next commit (commit 0)
was selected, for a total of 1 commit in the topmost 100 commits.
If instead the 250th commit was selected, then by the same logic
commit 50 is selected. At that point nextSelectionDistance() switches to
selecting consecutive commits, so commits 0-50 in the topmost 100
commits are selected. The "contiguousCommitCount" parameter provides
more determinism by always selecting a constant number or topmost
commits.
Add an optimization to break out of the inner loop of selectCommits() if
all of the commits for the current branch have already been found.
When reusing bitmaps from an existing pack, remove unnecessary
populating and clearing of the writeBitmaps/PackBitmapIndexBuilder.
Add comments to PackWriterBitmapPreparer, rename methods and variables
for readability.
Add tests for bitmap selection with and without merge commits and with
excessive branch pruning triggered.
Note: I will follow up with an additional change that exposes the new
parameters through PackConfig.
Change-Id: I5ccbb96c8849f331c302d9f7840e05f9650c4608 Signed-off-by: Terry Parker <tparker@google.com>
Matthias Sohn [Sun, 18 Oct 2015 16:28:04 +0000 (18:28 +0200)]
Silence Maven complaining about unset versions of reporting plugins
Since we use the reporting plugins only in the parent pom.xml there's no
point in using the new pluginManagement tag in the reporting section
which was introduced to fix
https://issues.apache.org/jira/browse/MSITE-443
Change-Id: I750ca3765e95afb06609a362fb3354afc3b66b90 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Andrei Pozolotin [Fri, 25 Sep 2015 20:55:32 +0000 (20:55 +0000)]
Adding JGitV1 and JGitV2 Walk Encryption
Building on top of https://git.eclipse.org/r/#/c/56391/
Here we preserve compatibility with JetS3t
and add 2 new native JGit encryption implementations.
For reference, see connection configuration files:
* Version 0: jgit-s3-connection-v-0.properties
* Version 1: jgit-s3-connection-v-1.properties
* Version 2: jgit-s3-connection-v-2.properties
Change-Id: I713290bcacbe92d88e5ef28ce137de73dd1abe2f Signed-off-by: Andrei Pozolotin <andrei.pozolotin@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Andrei Pozolotin [Mon, 21 Sep 2015 22:59:14 +0000 (22:59 +0000)]
Adding AES Walk Encryption support in http://www.jets3t.org/ mode
See previous attempt: https://git.eclipse.org/r/#/c/16674/
Here we preserve as much of JetS3t mode as possible
while allowing to use new Java 8+ PBE algorithms
such as PBEWithHmacSHA512AndAES_256
Summary of changes:
* change pom.xml to control long tests
* add WalkEncryptionTest.launch to run long tests
* add AmazonS3.Keys to to normalize use of constants
* change WalkEncryption to support AES in JetS3t mode
* add WalkEncryptionTest to test remote encryption pipeline
* add support for CI configuration for live Amazon S3 testing
* add log4j based logging for tests in both Eclipse and Maven build
To test locally, check out the review branch, then:
* create amazon test configuration file
* located your home dir: ${user.home}
* named jgit-s3-config.properties
* file format follows AmazonS3 connection settings file:
accesskey = your-amazon-access-key
secretkey = your-amazon-secret-key
test.bucket = your-bucket-for-testing
* finally:
* run in Eclipse: WalkEncryptionTest.launch
* or
* run in Shell: mvn test --define test=WalkEncryptionTest
Change-Id: I6f455fd9fb4eac261ca73d0bec6a4e7dae9f2e91 Signed-off-by: Andrei Pozolotin <andrei.pozolotin@gmail.com>
Andrey Loskutov [Sat, 10 Oct 2015 19:02:24 +0000 (22:02 +0300)]
Fixed jgit test failures on Windows
RepoCommandTest was failing because of open file handle left.
IgnoreNodeTest was failing because of problems with creation of files
with trailing spaces on Windows.
HookTest was failing because of wrong line delimiter.
Andrey Loskutov [Sun, 16 Aug 2015 11:00:00 +0000 (13:00 +0200)]
Delete non empty directories before checkout a path
If the checkout path is currently a non-empty directory (and was a link
or a regular file before), this directory will be removed before
performing checkout, but only if the checkout path is specified.
Terry Parker [Thu, 8 Oct 2015 22:06:37 +0000 (15:06 -0700)]
Limit the range of commits for which bitmaps are created.
A bitmap index contains bitmaps for a set of commits in a pack file.
Creating a bitmap for every commit is too expensive, so heuristics
select the most "important" commits. The most recent commits are the
most valuable. To clone a repository only those for the branch tips are
needed. When fetching, only commits since the last fetch are needed.
The commit selection heuristics generally work, but for some
repositories the number of selected commits is prohibitively high. One
example is the MSM 3.10 Linux kernel. With over 1 million commits on
2820 branches, the current heuristics resulted in +36k selected commits.
Each uncompressed bitmap for that repository is ~413k, making it
difficult to complete a GC operation in available memory.
The benefit of creating bitmaps over the entire history of a repository
like the MSM 3.10 Linux kernel isn't clear. For that repository, most
history for the last year appears to be in the last 100k commits.
Limiting bitmap commit selection to just those commits reduces the count
of selected commits from ~36k to ~10.5k. Dropping bitmaps for older
commits does not affect object counting times for clones or for fetches
on clients that are reasonably up-to-date.
This patch defines a new "bitmapCommitRange" PackConfig parameter to
limit the commit selection process when building bitmaps. The range
starts with the most recent commit and walks backwards. A range of 10k
considers only the 10000 most recent commits. A range of zero creates
bitmaps only for branch tips. A range of -1 (the default) does not limit
the range--all commits in the pack are used in the commit selection
process.
Change-Id: Ied92c70cfa0778facc670e0f14a0980bed5e3bfb Signed-off-by: Terry Parker <tparker@google.com>
Add utility method allowing to check for empty folders in workdir
Previously the method DirCacheCheckoutTest#assertWorkDir() silently
skipped over empty folders. If tests would have left unexpected empty
folders in the worktree this would be overlooked. Now empty folders have
to be specified by something like mkmap("<foldername>", "/", ...]
Change-Id: Idb8b270e92daf02ecdc381d148a5958bd83ec057 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Stefan Beller [Mon, 5 Oct 2015 23:01:11 +0000 (16:01 -0700)]
RepoCommand: Add setRecordRemoteBranch option to record upstream branch
On a server also running Gerrit that is using RepoCommand to
convert from an XML manifest to a git submodule superproject
periodically, it would be handy to be able to use Gerrit's
submodule subscription feature[1] to update the superproject
automatically between RepoCommand runs as changes are merged
in each subprojects.
This requires setting the 'branch' field for each submodule
so that Gerrit knows what branch to watch. Add an option to
do that.
Setting the branch field also is useful for plain Git users,
since it allows them to use "git submodule update --remote" to
manually update all submodules between RepoCommand runs.
Matthias Sohn [Thu, 1 Oct 2015 23:34:45 +0000 (01:34 +0200)]
Merge branch 'stable-4.1'
* stable-4.1:
pgm: Open RevWalk and TreeWalk in try-with-resource
ant: Open Repository and Git in try-with-resource
pgm: Create instances of Git in try-with-resource
FanoutBucket: Create ObjectInserter.Formatter in try-with-resource
Fix compiler warnings in DiffFormatter.writeGitLinkText
Change-Id: I448ecc9a1334977d9f304dd61ea20c7a8e692b10 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
[ignore rules] fix for handling unmatched '[' brackets
This patch makes JGit parsing of ignore rules containing unmatched '['
bracket compatible to the Git CLI.
Since '[' starts character group, Git tries to parse the ignore rule as
a shell glob pattern and if the character group is not closed, the glob
pattern is invalid and so the ignore rule never matches anything.
See also http://article.gmane.org/gmane.comp.version-control.git/278699.
Matthias Sohn [Mon, 21 Sep 2015 23:17:18 +0000 (01:17 +0200)]
Use java.nio.file consistently in FS
Since 4.0 we require Java 7 so there is no longer a need to override the
following methods in FS_POSIX, FS_Win32, FS_Win32_Cygwin
- lastModified()
- setLastModified()
- length()
- isSymlink()
- exists()
- isDirectory()
- isFile()
- isHidden()
Hence implement these methods in FS and remove overrides in subclasses.
Change-Id: I5dbde6ec806c66c86ac542978918361461021294 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
As discussed on https://git.eclipse.org/r/53836 it does not make sense
to have two similar utility classes in same package with intersecting
functionality. To not break the API, all methods from FileUtil are
copied to FileUtils, all FileUtil API is made deprecated and redirecting
now to FileUtils. Moved simple methods which are available in Java 7 API
are made package private and can be removed at any point later entirely
(right now they are in use).
SystemReader.isMacOs() and SystemReader.isWindows() return values are
unlikely to change during the JVM lifetime (except tests). Don't read
system properties each time the methods are called, just use previously
calculated value.
An attempt to re-implement not well documented Git CLI behavior for
patterns with backslashes.
It looks like Git silently ignores all \ characters in ignore rules, if
they are NOT covered by 3 cases described in [1]:
{quote}
1) ... Put a backslash ("\") in front of the first hash for patterns
that begin with a hash.
...
2) Trailing spaces are ignored unless they are quoted with backslash
("\").
...
3) Put a backslash ("\") in front of the first "!" for patterns that
begin with a literal "!", for example, "\!important!.txt".
{quote}
Undocumented also is the fact that backslash itself can be escaped by
backslash.
So \h\e\l\l\o\.t\x\t rule matches hello.txt and a\\\\b a\b in Git CLI.
Additionally, the glob parser [2] knows special meaning of backslash:
{quote}
One can remove the special meaning of '?', '*' and '[' by preceding
them by a backslash, or, in case this is part of a shell command
line, enclosing them in quotes. Between brackets these characters
stand for themselves. Thus, "[[?*\]" matches the four characters
'[', '?', '*' and '\'.
{quote}
Jacob Keller [Fri, 18 Sep 2015 06:05:07 +0000 (23:05 -0700)]
Show submodule difference as a hunk
Current DiffFormat behavior regarding submodules (aka git links) is
incorrect. The "Subproject commit <sha1>" appears as part of the diff
header, rather than as its own hunk.
The current way of processing submodules results in no hunk header and
includes the contents of the hunk as part of the headers. To fix this, we
can't just have our writeGitLinkDiffText output the hunk header. We have
to change the flow so that the raw text gets parsed as a diff. The easiest
way to do this is to fake the RawText in the FormatResult when we have a
GITLINK.
It should be noted that it seems possible for there to be a difference
between a GITLINK and a non-GITLINK, but I don't think this can happen in
practice, so I don't think we need to worry too much about it.
This patch also fixes up the test for GitLink headers, as the test was
for the old behavior. My setup has 3 other failing tests which may or
may not be the result of environmental changes. However, the same tests
fail without this commit, so I do not believe they are related.
Bug: 477759
Change-Id: If13f7b406904fad814416c93ed09ea47ef183337 Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Properly support special regex characters in ignore rules
Ignore rules should escape $^(){}+| chars if using regular expressions,
because they should be treated literally if they aren't part of a
character group.
UploadPack: Verify clients send only commits for shallow lines
If a client mistakenly tries to send a tag object as a shallow line
JGit blindly assumes this is a commit and tries to parse the tag
buffer using the commit parser. This can cause an obtuse error like:
InvalidObjectIdException: Invalid id: t c0ff331234...
The "t" comes from the "object c0ff331234..." line of the tag tring
to be parsed as though it where the "tree" line of a commit.
Run any client supplied shallow lines through the RevWalk to lookup
the object types. Fail fast with a protocol exception if any of them
are non-commit.
Skip objects not known to this repository. This matches behavior
with git-core's upload-pack, which sliently skips over any shallow
line object named by the client but not known by the server.
When we have a URI that contains an empty path component (that is
it only contains a "/") we want to fall back to the host as
humanish name.
This change is according to the behavior of upstream git, which
falls back on the hostname when guessing directory names for
newly cloned repositories (see [1] for the discussion).
Matthias Sohn [Mon, 13 Jul 2015 23:55:58 +0000 (01:55 +0200)]
Update org.apache.httpcomponents
- update org.apache.httpcomponents.httpcore to 4.3.3
- update org.apache.httpcomponents.httpclient to 4.3.6, 4.3.5 and later
are reported to fix vulnerability CVE-2014-3577
Dave Borowitz [Wed, 2 Sep 2015 19:04:33 +0000 (15:04 -0400)]
PushCertificateStore: Don't add no-op command to batch
If no refs match the input list and we are writing to a batch,
the returned new commit from write() will match the current commit.
Adding a command to the batch for this case is harmless as it will
succeed, but it's more straightforward to just skip adding a command
in this case.
Add tests or the combination of saving matching refs and saving to a
batch.
Matthias Sohn [Mon, 31 Aug 2015 07:38:18 +0000 (09:38 +0200)]
Update uses-clauses in OSGi manifests
In Bug 476164 it was reported that EGit doesn't start when the platform
comes with jsch 0.1.51 while this version of EGit/JGit brings jsch
0.1.53. This could be caused by outdated uses-clauses. Hence recompute
them using PDE tooling.
Bug: 476164
Change-Id: I185ba097884ead9cd034eba842bd3bf34181a99b Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Fri, 28 Aug 2015 11:09:50 +0000 (13:09 +0200)]
Use java.io.File to check existence of loose objects in ObjectDirectory
It was reported in [1] that 197e3393a51424fae45e51dce4a649ba26e5a368 led
to a performance regression in a BFG benchmark. Analysis showed that
this is caused by the exists() method in FS_POSIX, now overriding the
default implementation in FS. The default implementation of FS.exists()
uses java.io.File.exists(), while the new implementation in FS_POSIX
uses java.nio.file.Files.exists() - by simply removing the override in
FS_POSIX, performance was restored.
Profiling showed that java.nio.file.Files.exists() is substantially
slower than java.io.File.exists(), to the point where the exists() call
doubles the average cost of a call to
ObjectDirectory.insertUnpackedObject() - which the BFG uses a lot,
because it's rewriting history. Average times measured on Ubuntu were:
The loose object exists test should be using java.io.File and not FS.
ObjectDirectory uses FS.resolve() to traverse symlinks to objects but
then once inside objects all 256 sharded directories should be real
directories, and the object files should be real files, not dangling
symlinks. java.io.File.exists() is sufficient here, and faster.
Change ObjectDirectory to use File.exists() once its computed the File
handle.
This does mean JGit cannot run ObjectDirectory code on an abstract
virtual filesystem plugged into NIO2. If you really want to run JGit on
an esoteric non-standard filesystem like "in memory" you should look at
the DFS storage backend, which has fewer abstraction points to deal
with. Or write your own from scratch.
Martin Fick [Wed, 19 Aug 2015 21:05:54 +0000 (15:05 -0600)]
Handle stale file handles on packed-refs file
On a local filesystem the packed-refs file will be orphaned if it is
replaced by another client while the current client is reading the old
one. However, since NFS servers do not keep track of open files, instead
of orphaning the old packed-refs file, such a replacement will cause the
old file to be garbage collected instead. A stale file handle exception
will be raised on NFS servers if the file is garbage collected (deleted)
on the server while it is being read. Since we no longer have access to
the old file in these cases, the previous code would just fail. However,
in these cases, reopening the file and rereading it will succeed (since
it will reopen the new replacement file). So retrying the read is a
viable strategy to deal with stale file handles on the packed-refs file,
implement such a strategy.
Since it is possible that the packed-refs file could be replaced again
while rereading it (multiple consecutive updates can easily occur with
ref deletions), loop on stale file handle exceptions, up to 5 extra
times, trying to read the packed-refs file again, until we either read
the new file, or find that the file no longer exists. The limit of 5 is
arbitrary, and provides a safe upper bounds to prevent infinite loops
consuming resources in a potential unforeseen persistent error
condition.
Change-Id: I085c472bafa6e2f32f610a33ddc8368bb4ab1814 Signed-off-by: Martin Fick<mfick@codeaurora.org> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Martin Fick [Tue, 25 Aug 2015 13:48:50 +0000 (07:48 -0600)]
Add public isStaleFileHandle() API, improve detection.
Add a public API to the FileUtils to determine if an IOException is a
stale NFS file handle exception. This will make it easier to detect
such errors, and interpret them consistently throughout the codebase.
This new API is a bit more lenient in its detection than the previous
detection, and should be able to detect some errors which previously
were not identified as stale file handle exceptions because they had the
word NFS in the error message. Adjust the packfile handling code to use
this new API for detection.
Change-Id: I21f80014546ba1afec7335890e5ae79e7f521412 Signed-off-by: Martin Fick<mfick@codeaurora.org>
Set "potentialNullReference" to "error" level and fixed all issues
There should be no functional change, the logic updated only to make
code simple so that compiler can understand what is going for. Removed
all @SuppressWarnings("null") annotations since they cannot be used if
"org.eclipse.jdt.core.compiler.problem.potentialNullReference" option is
set to the "error" level.
Matthias Sohn [Wed, 19 Aug 2015 13:48:12 +0000 (15:48 +0200)]
Update com.jcraft.jsch to 0.1.53
Update target platform to Orbit M20150818205559 for Mars in order to
update com.jcraft.jsch to 0.1.53. Also update pom.xml to use Mars target
platform profile by default.
CQ: 10045
Bug: 463580
Change-Id: I1bf151fbee7b00c7bd38cf1236c9bad50e3c64bd Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
projects / jgit.git / log