Browse Source

Use java.io.File to check existence of loose objects in ObjectDirectory

It was reported in [1] that 197e3393a5 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:

java.io.File.exists() - 4 microseconds
java.nio.file.Files.exists() - 60 microseconds

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.

[1] https://dev.eclipse.org/mhonarc/lists/jgit-dev/msg02954.html

Change-Id: I74684dc3957ae1ca52a7097f83a6c420aa24310f
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
tags/v4.1.0.201509280440-r
Matthias Sohn 8 years ago
parent
commit
05acf1c62f

+ 1
- 1
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java View File

@@ -599,7 +599,7 @@ public class ObjectDirectory extends FileObjectDatabase {
}

final File dst = fileFor(id);
if (fs.exists(dst)) {
if (dst.exists()) {
// We want to be extra careful and avoid replacing an object
// that already exists. We can't be sure renameTo() would
// fail on all platforms if dst exists, so we check first.

Loading…
Cancel
Save