Merge branch 'fs-fsync'

* fs-fsync:
  Remove unnecessary flush calls from LockFile
  Remove unnecessary region locking from LockFile
  Support core.fsyncRefFiles option
  Support core.fsyncObjectFiles option
  Simplify LockFile write(ObjectId) case

5 files changed, 173 insertions, 76 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java
index e8bc3e2cfd..6199f4c450 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java
@@ -44,7 +44,6 @@
 
 package org.eclipse.jgit.storage.file;
 
-import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -52,8 +51,9 @@ import java.io.FileOutputStream;
 import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.nio.channels.FileLock;
-import java.nio.channels.OverlappingFileLockException;
+import java.nio.ByteBuffer;
+import java.nio.channels.Channels;
+import java.nio.channels.FileChannel;
 import java.text.MessageFormat;
 
 import org.eclipse.jgit.JGitText;
@@ -85,14 +85,14 @@ public class LockFile {
 
 	private final File lck;
 
-	private FileLock fLck;
-
 	private boolean haveLck;
 
 	private FileOutputStream os;
 
 	private boolean needStatInformation;
 
+	private boolean fsync;
+
 	private long commitLastModified;
 
 	private final FS fs;
@@ -127,23 +127,6 @@ public class LockFile {
 			haveLck = true;
 			try {
 				os = new FileOutputStream(lck);
-				try {
-					fLck = os.getChannel().tryLock();
-					if (fLck == null)
-						throw new OverlappingFileLockException();
-				} catch (OverlappingFileLockException ofle) {
-					// We cannot use unlock() here as this file is not
-					// held by us, but we thought we created it. We must
-					// not delete it, as it belongs to some other process.
-					//
-					haveLck = false;
-					try {
-						os.close();
-					} catch (IOException ioe) {
-						// Fail by returning haveLck = false.
-					}
-					os = null;
-				}
 			} catch (IOException ioe) {
 				unlock();
 				throw ioe;
@@ -192,10 +175,21 @@ public class LockFile {
 		try {
 			final FileInputStream fis = new FileInputStream(ref);
 			try {
-				final byte[] buf = new byte[2048];
-				int r;
-				while ((r = fis.read(buf)) >= 0)
-					os.write(buf, 0, r);
+				if (fsync) {
+					FileChannel in = fis.getChannel();
+					long pos = 0;
+					long cnt = in.size();
+					while (0 < cnt) {
+						long r = os.getChannel().transferFrom(in, pos, cnt);
+						pos += r;
+						cnt -= r;
+					}
+				} else {
+					final byte[] buf = new byte[2048];
+					int r;
+					while ((r = fis.read(buf)) >= 0)
+						os.write(buf, 0, r);
+				}
 			} finally {
 				fis.close();
 			}
@@ -229,26 +223,10 @@ public class LockFile {
 	 *             before throwing the underlying exception to the caller.
 	 */
 	public void write(final ObjectId id) throws IOException {
-		requireLock();
-		try {
-			final BufferedOutputStream b;
-			b = new BufferedOutputStream(os, Constants.OBJECT_ID_STRING_LENGTH + 1);
-			id.copyTo(b);
-			b.write('\n');
-			b.flush();
-			fLck.release();
-			b.close();
-			os = null;
-		} catch (IOException ioe) {
-			unlock();
-			throw ioe;
-		} catch (RuntimeException ioe) {
-			unlock();
-			throw ioe;
-		} catch (Error ioe) {
-			unlock();
-			throw ioe;
-		}
+		byte[] buf = new byte[Constants.OBJECT_ID_STRING_LENGTH + 1];
+		id.copyTo(buf, 0);
+		buf[Constants.OBJECT_ID_STRING_LENGTH] = '\n';
+		write(buf);
 	}
 
 	/**
@@ -268,9 +246,15 @@ public class LockFile {
 	public void write(final byte[] content) throws IOException {
 		requireLock();
 		try {
-			os.write(content);
-			os.flush();
-			fLck.release();
+			if (fsync) {
+				FileChannel fc = os.getChannel();
+				ByteBuffer buf = ByteBuffer.wrap(content);
+				while (0 < buf.remaining())
+					fc.write(buf);
+				fc.force(true);
+			} else {
+				os.write(content);
+			}
 			os.close();
 			os = null;
 		} catch (IOException ioe) {
@@ -296,34 +280,36 @@ public class LockFile {
 	 */
 	public OutputStream getOutputStream() {
 		requireLock();
+
+		final OutputStream out;
+		if (fsync)
+			out = Channels.newOutputStream(os.getChannel());
+		else
+			out = os;
+
 		return new OutputStream() {
 			@Override
 			public void write(final byte[] b, final int o, final int n)
 					throws IOException {
-				os.write(b, o, n);
+				out.write(b, o, n);
 			}
 
 			@Override
 			public void write(final byte[] b) throws IOException {
-				os.write(b);
+				out.write(b);
 			}
 
 			@Override
 			public void write(final int b) throws IOException {
-				os.write(b);
-			}
-
-			@Override
-			public void flush() throws IOException {
-				os.flush();
+				out.write(b);
 			}
 
 			@Override
 			public void close() throws IOException {
 				try {
-					os.flush();
-					fLck.release();
-					os.close();
+					if (fsync)
+						os.getChannel().force(true);
+					out.close();
 					os = null;
 				} catch (IOException ioe) {
 					unlock();
@@ -357,6 +343,16 @@ public class LockFile {
 	}
 
 	/**
+	 * Request that {@link #commit()} force dirty data to the drive.
+	 *
+	 * @param on
+	 *            true if dirty data should be forced to the drive.
+	 */
+	public void setFSync(final boolean on) {
+		fsync = on;
+	}
+
+	/**
 	 * Wait until the lock file information differs from the old file.
 	 * <p>
 	 * This method tests both the length and the last modification date. If both
@@ -447,14 +443,6 @@ public class LockFile {
 	 */
 	public void unlock() {
 		if (os != null) {
-			if (fLck != null) {
-				try {
-					fLck.release();
-				} catch (IOException ioe) {
-					// Huh?
-				}
-				fLck = null;
-			}
 			try {
 				os.close();
 			} catch (IOException ioe) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java
index 16cb8aa35c..d922bebe8f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java
@@ -52,6 +52,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.channels.Channels;
 import java.security.DigestOutputStream;
 import java.security.MessageDigest;
 import java.util.zip.Deflater;
@@ -60,7 +61,6 @@ import java.util.zip.DeflaterOutputStream;
 import org.eclipse.jgit.errors.ObjectWritingException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.CoreConfig;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 
@@ -68,13 +68,13 @@ import org.eclipse.jgit.lib.ObjectInserter;
 class ObjectDirectoryInserter extends ObjectInserter {
 	private final FileObjectDatabase db;
 
-	private final Config config;
+	private final WriteConfig config;
 
 	private Deflater deflate;
 
 	ObjectDirectoryInserter(final FileObjectDatabase dest, final Config cfg) {
 		db = dest;
-		config = cfg;
+		config = cfg.get(WriteConfig.KEY);
 	}
 
 	@Override
@@ -121,9 +121,13 @@ class ObjectDirectoryInserter extends ObjectInserter {
 		boolean delete = true;
 		File tmp = newTempFile();
 		try {
-			DigestOutputStream dOut = new DigestOutputStream(
-					compress(new FileOutputStream(tmp)), md);
+			FileOutputStream fOut = new FileOutputStream(tmp);
 			try {
+				OutputStream out = fOut;
+				if (config.getFSyncObjectFiles())
+					out = Channels.newOutputStream(fOut.getChannel());
+				DeflaterOutputStream cOut = compress(out);
+				DigestOutputStream dOut = new DigestOutputStream(cOut, md);
 				writeHeader(dOut, type, len);
 
 				final byte[] buf = buffer();
@@ -134,8 +138,12 @@ class ObjectDirectoryInserter extends ObjectInserter {
 					dOut.write(buf, 0, n);
 					len -= n;
 				}
+				dOut.flush();
+				cOut.finish();
 			} finally {
-				dOut.close();
+				if (config.getFSyncObjectFiles())
+					fOut.getChannel().force(true);
+				fOut.close();
 			}
 
 			delete = false;
@@ -160,7 +168,7 @@ class ObjectDirectoryInserter extends ObjectInserter {
 
 	DeflaterOutputStream compress(final OutputStream out) {
 		if (deflate == null)
-			deflate = new Deflater(config.get(CoreConfig.KEY).getCompression());
+			deflate = new Deflater(config.getCompression());
 		else
 			deflate.reset();
 		return new DeflaterOutputStream(out, deflate);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
index 96c8361adb..2af7ca3e6d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
@@ -67,6 +67,8 @@ import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.LinkedList;
@@ -606,6 +608,7 @@ public class RefDirectory extends RefDatabase {
 			write = false;
 
 		if (write) {
+			WriteConfig wc = getRepository().getConfig().get(WriteConfig.KEY);
 			FileOutputStream out;
 			try {
 				out = new FileOutputStream(log, true);
@@ -618,7 +621,15 @@ public class RefDirectory extends RefDatabase {
 				out = new FileOutputStream(log, true);
 			}
 			try {
-				out.write(rec);
+				if (wc.getFSyncRefFiles()) {
+					FileChannel fc = out.getChannel();
+					ByteBuffer buf = ByteBuffer.wrap(rec);
+					while (0 < buf.remaining())
+						fc.write(buf);
+					fc.force(true);
+				} else {
+					out.write(rec);
+				}
 			} finally {
 				out.close();
 			}
@@ -757,6 +768,7 @@ public class RefDirectory extends RefDatabase {
 			@Override
 			protected void writeFile(String name, byte[] content)
 					throws IOException {
+				lck.setFSync(true);
 				lck.setNeedStatInformation(true);
 				try {
 					lck.write(content);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryUpdate.java
index a9f054837b..109960df2b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryUpdate.java
@@ -99,6 +99,10 @@ class RefDirectoryUpdate extends RefUpdate {
 
 	@Override
 	protected Result doUpdate(final Result status) throws IOException {
+		WriteConfig wc = database.getRepository().getConfig()
+				.get(WriteConfig.KEY);
+
+		lock.setFSync(wc.getFSyncRefFiles());
 		lock.setNeedStatInformation(true);
 		lock.write(getNewObjectId());
 
@@ -143,6 +147,10 @@ class RefDirectoryUpdate extends RefUpdate {
 
 	@Override
 	protected Result doLink(final String target) throws IOException {
+		WriteConfig wc = database.getRepository().getConfig()
+				.get(WriteConfig.KEY);
+
+		lock.setFSync(wc.getFSyncRefFiles());
 		lock.setNeedStatInformation(true);
 		lock.write(encode(RefDirectory.SYMREF + target + '\n'));
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WriteConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WriteConfig.java
new file mode 100644
index 0000000000..fd467a5554
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WriteConfig.java
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2010, Google Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.storage.file;
+
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Config.SectionParser;
+import org.eclipse.jgit.lib.CoreConfig;
+
+class WriteConfig {
+	/** Key for {@link Config#get(SectionParser)}. */
+	static final Config.SectionParser<WriteConfig> KEY = new SectionParser<WriteConfig>() {
+		public WriteConfig parse(final Config cfg) {
+			return new WriteConfig(cfg);
+		}
+	};
+
+	private final int compression;
+
+	private final boolean fsyncObjectFiles;
+
+	private final boolean fsyncRefFiles;
+
+	private WriteConfig(final Config rc) {
+		compression = rc.get(CoreConfig.KEY).getCompression();
+		fsyncObjectFiles = rc.getBoolean("core", "fsyncobjectfiles", false);
+		fsyncRefFiles = rc.getBoolean("core", "fsyncreffiles", false);
+	}
+
+	int getCompression() {
+		return compression;
+	}
+
+	boolean getFSyncObjectFiles() {
+		return fsyncObjectFiles;
+	}
+
+	boolean getFSyncRefFiles() {
+		return fsyncRefFiles;
+	}
+}