github
/
jgit
mirror of https://gitee.com/github-mirror-repo/jgit.git
1
0
Fork 0
Code Issues Releases Wiki Activity
Browse Source

Let RefDirectory use FileSnapShot to handle fast updates 

Since this change may affect performance and memory consumption on every
access to a loose ref I explicitly made it a RFC to collect opinions.

Previously RefDirectory.scanRef() was not detecting an update of a
loose ref when the update didn't changed the modification time of
the backing file. RefDirectory cached loose refs and the way to detect
outdated cache entries was to compare lastmodification timestamp on the
file representing the ref. If two updates to the same ref happen faster
than the filesystem-timer granularity (for linux this is 2 seconds)
there is the possiblity that we don't detect the update.

Because of this bug EGit's PushOperationTest only works with 2 second
sleeps inside.

This change let RefDirectory use FileSnapshot to detect such situations.
FileSnapshot helps to remember when a file was last read from disk and
therefore enables to decide when to load a file from disk although
modification time has not changed.

Change-Id: I03b9a137af097ec69c4c5e2eaa512d2bdd7fe080
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
stable-1.0
Christian Halstrick 14 years ago committed by Chris Aniszczyk
parent
16e810b2ec
commit
2302a6d3ce
4 changed files with 98 additions and 49 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 43
      org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
  2. 25
      org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
  3. 16
      org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java
  4. 63
      org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java

43
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
Unescape View File

@ -152,4 +152,47 @@ public class PushCommandTest extends RepositoryTestCase {
assertEquals(commit2.getId(), db2.resolve(branch));
}
/**
* Check that pushes over file protocol lead to appropriate ref-updates.
*
* @throws Exception
*/
@Test
public void testPushRefUpdate() throws Exception {
Git git = new Git(db);
Git git2 = new Git(createBareRepository());
final StoredConfig config = git.getRepository().getConfig();
RemoteConfig remoteConfig = new RemoteConfig(config, "test");
URIish uri = new URIish(git2.getRepository().getDirectory().toURI()
.toURL());
remoteConfig.addURI(uri);
remoteConfig.addPushRefSpec(new RefSpec("+refs/heads/*:refs/heads/*"));
remoteConfig.update(config);
config.save();
writeTrashFile("f", "content of f");
git.add().addFilepattern("f").call();
RevCommit commit = git.commit().setMessage("adding f").call();
assertEquals(null, git2.getRepository().resolve("refs/heads/master"));
git.push().setRemote("test").call();
assertEquals(commit.getId(),
git2.getRepository().resolve("refs/heads/master"));
git.branchCreate().setName("refs/heads/test").call();
git.checkout().setName("refs/heads/test").call();
for (int i = 0; i < 6; i++) {
writeTrashFile("f" + i, "content of f" + i);
git.add().addFilepattern("f" + i).call();
commit = git.commit().setMessage("adding f" + i).call();
git.push().setRemote("test").call();
git2.getRepository().getAllRefs();
assertEquals("failed to update on attempt " + i, commit.getId(),
git2.getRepository().resolve("refs/heads/test"));
}
}
}

25
org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
Unescape View File

@ -169,7 +169,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
Ref head;
writeLooseRef(HEAD, A);
BUG_WorkAroundRacyGitIssues(HEAD);
all = refdir.getRefs(RefDatabase.ALL);
assertEquals(1, all.size());
@ -190,7 +189,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
writeLooseRef(HEAD, A);
writeLooseRef("refs/heads/master", B);
BUG_WorkAroundRacyGitIssues(HEAD);
all = refdir.getRefs(RefDatabase.ALL);
assertEquals(2, all.size());
@ -328,7 +326,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
assertTrue(heads.containsKey("refs/heads/C"));
writeLooseRef("refs/heads/B", "FAIL\n");
BUG_WorkAroundRacyGitIssues("refs/heads/B");
heads = refdir.getRefs(RefDatabase.ALL);
assertEquals(2, heads.size());
@ -547,7 +544,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
assertEquals(A, all.get(HEAD).getObjectId());
writeLooseRef("refs/heads/master", B);
BUG_WorkAroundRacyGitIssues("refs/heads/master");
all = refdir.getRefs(RefDatabase.ALL);
assertEquals(B, all.get(HEAD).getObjectId());
}
@ -561,7 +557,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
assertEquals(A, all.get(HEAD).getObjectId());
writeLooseRef("refs/heads/master", B);
BUG_WorkAroundRacyGitIssues("refs/heads/master");
Ref master = refdir.getRef("refs/heads/master");
assertEquals(B, master.getObjectId());
@ -760,7 +755,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
writeLooseRef("refs/5", "ref: refs/6\n");
writeLooseRef("refs/6", "ref: refs/end\n");
BUG_WorkAroundRacyGitIssues("refs/5");
all = refdir.getRefs(RefDatabase.ALL);
r = all.get("refs/1");
assertNull("mising 1 due to cycle", r);
@ -1078,23 +1072,4 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
File path = new File(diskRepo.getDirectory(), name);
assertTrue("deleted " + name, path.delete());
}
/**
* Kick the timestamp of a local file.
* <p>
* We shouldn't have to make these method calls. The cache is using file
* system timestamps, and on many systems unit tests run faster than the
* modification clock. Dumping the cache after we make an edit behind
* RefDirectory's back allows the tests to pass.
*
* @param name
* the file in the repository to force a time change on.
*/
private void BUG_WorkAroundRacyGitIssues(String name) {
File path = new File(diskRepo.getDirectory(), name);
long old = path.lastModified();
long set = 1250379778668L; // Sat Aug 15 20:12:58 GMT-03:30 2009
path.setLastModified(set);
assertTrue("time changed", old != path.lastModified());
}
}

16
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java
Unescape View File

@ -101,6 +101,22 @@ public class FileSnapshot {
return new FileSnapshot(read, modified);
}
/**
* Record a snapshot for a file for which the last modification time is
* already known.
* <p>
* This method should be invoked before the file is accessed.
*
* @param modified
* the last modification time of the file
*
* @return the snapshot.
*/
public static FileSnapshot save(long modified) {
final long read = System.currentTimeMillis();
return new FileSnapshot(read, modified);
}
/** Last observed modification time of the path. */
private final long lastModified;

63
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
Unescape View File

@ -842,19 +842,22 @@ public class RefDirectory extends RefDatabase {
return n;
}
@SuppressWarnings("null")
private LooseRef scanRef(LooseRef ref, String name) throws IOException {
final File path = fileFor(name);
final long modified = path.lastModified();
FileSnapshot currentSnapshot = null;
if (ref != null) {
if (ref.getLastModified() == modified)
currentSnapshot = ref.getSnapShot();
if (!currentSnapshot.isModified(path))
return ref;
name = ref.getName();
} else if (modified == 0)
} else if (!path.exists())
return null;
final int limit = 4096;
final byte[] buf;
FileSnapshot otherSnapshot = FileSnapshot.save(path);
try {
buf = IO.readSome(path, limit);
} catch (FileNotFoundException noFile) {
@ -877,7 +880,12 @@ public class RefDirectory extends RefDatabase {
throw new IOException(MessageFormat.format(JGitText.get().notARef, name, content));
}
final String target = RawParseUtils.decode(buf, 5, n);
return newSymbolicRef(modified, name, target);
if (ref != null && ref.isSymbolic()
&& ref.getTarget().getName().equals(target)) {
currentSnapshot.setClean(otherSnapshot);
return ref;
}
return newSymbolicRef(path.lastModified(), name, target);
}
if (n < OBJECT_ID_STRING_LENGTH)
@ -886,13 +894,19 @@ public class RefDirectory extends RefDatabase {
final ObjectId id;
try {
id = ObjectId.fromString(buf, 0);
if (ref != null && !ref.isSymbolic()
&& ref.getTarget().getObjectId().equals(id)) {
currentSnapshot.setClean(otherSnapshot);
return ref;
}
} catch (IllegalArgumentException notRef) {
while (0 < n && Character.isWhitespace(buf[n - 1]))
n--;
String content = RawParseUtils.decode(buf, 0, n);
throw new IOException(MessageFormat.format(JGitText.get().notARef, name, content));
}
return new LooseUnpeeled(modified, name, id);
return new LooseUnpeeled(path.lastModified(), name, id);
}
private static boolean isSymRef(final byte[] buf, int n) {
@ -997,22 +1011,22 @@ public class RefDirectory extends RefDatabase {
}
private static interface LooseRef extends Ref {
long getLastModified();
FileSnapshot getSnapShot();
LooseRef peel(ObjectIdRef newLeaf);
}
private final static class LoosePeeledTag extends ObjectIdRef.PeeledTag
implements LooseRef {
private final long lastModified;
private final FileSnapshot snapShot;
LoosePeeledTag(long mtime, String refName, ObjectId id, ObjectId p) {
super(LOOSE, refName, id, p);
this.lastModified = mtime;
snapShot = FileSnapshot.save(mtime);
}
public long getLastModified() {
return lastModified;
public FileSnapshot getSnapShot() {
return snapShot;
}
public LooseRef peel(ObjectIdRef newLeaf) {
@ -1022,15 +1036,15 @@ public class RefDirectory extends RefDatabase {
private final static class LooseNonTag extends ObjectIdRef.PeeledNonTag
implements LooseRef {
private final long lastModified;
private final FileSnapshot snapShot;
LooseNonTag(long mtime, String refName, ObjectId id) {
super(LOOSE, refName, id);
this.lastModified = mtime;
snapShot = FileSnapshot.save(mtime);
}
public long getLastModified() {
return lastModified;
public FileSnapshot getSnapShot() {
return snapShot;
}
public LooseRef peel(ObjectIdRef newLeaf) {
@ -1040,37 +1054,38 @@ public class RefDirectory extends RefDatabase {
private final static class LooseUnpeeled extends ObjectIdRef.Unpeeled
implements LooseRef {
private final long lastModified;
private final FileSnapshot snapShot;
LooseUnpeeled(long mtime, String refName, ObjectId id) {
super(LOOSE, refName, id);
this.lastModified = mtime;
snapShot = FileSnapshot.save(mtime);
}
public long getLastModified() {
return lastModified;
public FileSnapshot getSnapShot() {
return snapShot;
}
public LooseRef peel(ObjectIdRef newLeaf) {
if (newLeaf.getPeeledObjectId() != null)
return new LoosePeeledTag(lastModified, getName(),
return new LoosePeeledTag(snapShot.lastModified(), getName(),
getObjectId(), newLeaf.getPeeledObjectId());
else
return new LooseNonTag(lastModified, getName(), getObjectId());
return new LooseNonTag(snapShot.lastModified(), getName(),
getObjectId());
}
}
private final static class LooseSymbolicRef extends SymbolicRef implements
LooseRef {
private final long lastModified;
private final FileSnapshot snapShot;
LooseSymbolicRef(long mtime, String refName, Ref target) {
super(refName, target);
this.lastModified = mtime;
snapShot = FileSnapshot.save(mtime);
}
public long getLastModified() {
return lastModified;
public FileSnapshot getSnapShot() {
return snapShot;
}
public LooseRef peel(ObjectIdRef newLeaf) {

Write Preview
Loading…
Cancel
Save