You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by th...@apache.org on 2010/10/13 15:21:11 UTC
svn commit: r1022094 - in
/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core:
data/ value/
Author: thomasm
Date: Wed Oct 13 13:21:11 2010
New Revision: 1022094
URL: http://svn.apache.org/viewvc?rev=1022094&view=rev
Log:
JCR-2781 FileDataStore performance improvements
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BinaryValueImpl.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/ValueFactoryImpl.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java?rev=1022094&r1=1022093&r2=1022094&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java Wed Oct 13 13:21:11 2010
@@ -131,20 +131,38 @@ public class FileDataStore implements Da
}
public DataRecord getRecordIfStored(DataIdentifier identifier) throws DataStoreException {
+ return getRecord(identifier, true);
+ }
+
+ /**
+ * Get a data record for the given identifier.
+ * This method only checks if the file exists if the verify flag is set.
+ * If the verify flag is set and the file doesn't exist, the method returns null.
+ *
+ * @param identifier the identifier
+ * @param verify whether to check if the file exists
+ * @return the data record or null
+ */
+ private DataRecord getRecord(DataIdentifier identifier, boolean verify) throws DataStoreException {
File file = getFile(identifier);
synchronized (this) {
- if (!file.exists()) {
+ if (verify && !file.exists()) {
return null;
}
- if (minModifiedDate != 0 && file.canWrite()) {
+ if (minModifiedDate != 0) {
+ // only check when running garbage collection
long lastModified = file.lastModified();
if (lastModified == 0) {
throw new DataStoreException("Failed to read record modified date: " + identifier);
- }
+ }
if (lastModified < minModifiedDate) {
- // The current GC approach depends on this call succeeding
- if (!file.setLastModified(System.currentTimeMillis() + ACCESS_TIME_RESOLUTION)) {
- throw new DataStoreException("Failed to update record modified date: " + identifier);
+ // in most cases we can write to the file system, that's why we
+ // don't call File.canWrite earlier (to save a system call)
+ if (file.canWrite()) {
+ // The current GC approach depends on this call succeeding
+ if (!file.setLastModified(System.currentTimeMillis() + ACCESS_TIME_RESOLUTION)) {
+ throw new DataStoreException("Failed to update record modified date: " + identifier);
+ }
}
}
}
@@ -163,11 +181,7 @@ public class FileDataStore implements Da
* @return identified data record
*/
public DataRecord getRecord(DataIdentifier identifier) throws DataStoreException {
- DataRecord record = getRecordIfStored(identifier);
- if (record == null) {
- throw new DataStoreException("Record not found: " + identifier);
- }
- return record;
+ return getRecord(identifier, false);
}
private void usesIdentifier(DataIdentifier identifier) {
@@ -211,13 +225,13 @@ public class FileDataStore implements Da
// move the temporary file in place if needed
usesIdentifier(identifier);
file = getFile(identifier);
- File parent = file.getParentFile();
- if (!parent.isDirectory()) {
- parent.mkdirs();
- }
if (!file.exists()) {
- temporary.renameTo(file);
- if (!file.exists()) {
+ File parent = file.getParentFile();
+ parent.mkdirs();
+ if (temporary.renameTo(file)) {
+ // no longer need to delete the temporary file
+ temporary = null;
+ } else {
throw new IOException(
"Can not rename " + temporary.getAbsolutePath()
+ " to " + file.getAbsolutePath()
@@ -238,12 +252,12 @@ public class FileDataStore implements Da
}
}
}
- // Sanity checks on the record file. These should never fail,
- // but better safe than sorry...
- if (!file.isFile()) {
- throw new IOException("Not a file: " + file);
- }
if (file.length() != length) {
+ // Sanity checks on the record file. These should never fail,
+ // but better safe than sorry...
+ if (!file.isFile()) {
+ throw new IOException("Not a file: " + file);
+ }
throw new IOException(DIGEST + " collision: " + file);
}
}
@@ -289,9 +303,7 @@ public class FileDataStore implements Da
* @throws IOException
*/
private File newTemporaryFile() throws IOException {
- if (!directory.isDirectory()) {
- directory.mkdirs();
- }
+ // the directory is already created in the init method
return File.createTempFile(TMP, null, directory);
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java?rev=1022094&r1=1022093&r2=1022094&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java Wed Oct 13 13:21:11 2010
@@ -23,6 +23,7 @@ import javax.jcr.RepositoryException;
import javax.jcr.Binary;
import org.apache.jackrabbit.core.data.DataIdentifier;
+import org.apache.jackrabbit.core.data.DataStore;
/**
* Represents binary data which is backed by a resource or byte[].
@@ -92,4 +93,14 @@ abstract class BLOBFileValue implements
}
}
+ /**
+ * Check if this blob uses the given data store.
+ *
+ * @param s the other data store
+ * @return true if it does
+ */
+ boolean usesDataStore(DataStore s) {
+ return false;
+ }
+
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java?rev=1022094&r1=1022093&r2=1022094&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java Wed Oct 13 13:21:11 2010
@@ -135,4 +135,8 @@ class BLOBInDataStore extends BLOBFileVa
return store.getRecord(identifier);
}
+ boolean usesDataStore(DataStore s) {
+ return store == s;
+ }
+
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BinaryValueImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BinaryValueImpl.java?rev=1022094&r1=1022093&r2=1022094&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BinaryValueImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BinaryValueImpl.java Wed Oct 13 13:21:11 2010
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.core.value
import javax.jcr.RepositoryException;
import org.apache.jackrabbit.api.JackrabbitValue;
import org.apache.jackrabbit.core.data.DataIdentifier;
+import org.apache.jackrabbit.core.data.DataStore;
import org.apache.jackrabbit.value.BinaryValue;
/**
@@ -52,4 +53,8 @@ class BinaryValueImpl extends BinaryValu
return blob.getDataIdentifier();
}
+ boolean usesDataStore(DataStore s) {
+ return blob.usesDataStore(s);
+ }
+
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java?rev=1022094&r1=1022093&r2=1022094&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java Wed Oct 13 13:21:11 2010
@@ -120,12 +120,17 @@ public class InternalValue extends Abstr
BinaryValueImpl bin = (BinaryValueImpl) value;
DataIdentifier identifier = bin.getDataIdentifier();
if (identifier != null) {
- // access the record to ensure it is not garbage collected
- if (store.getRecordIfStored(identifier) != null) {
- // it exists - so we don't need to stream it again
- // but we need to create a new object because the original
- // one might be in a different data store (repository)
+ if (bin.usesDataStore(store)) {
+ // access the record to ensure it is not garbage collected
+ store.getRecord(identifier);
blob = BLOBInDataStore.getInstance(store, identifier);
+ } else {
+ if (store.getRecordIfStored(identifier) != null) {
+ // it exists - so we don't need to stream it again
+ // but we need to create a new object because the original
+ // one might be in a different data store (repository)
+ blob = BLOBInDataStore.getInstance(store, identifier);
+ }
}
}
}
@@ -253,16 +258,28 @@ public class InternalValue extends Abstr
return tmp;
}
- static InternalValue getInternalValue(DataIdentifier identifier, DataStore store) throws DataStoreException {
- // access the record to ensure it is not garbage collected
- if (store.getRecordIfStored(identifier) != null) {
- // it exists - so we don't need to stream it again
- // but we need to create a new object because the original
- // one might be in a different data store (repository)
- BLOBFileValue blob = BLOBInDataStore.getInstance(store, identifier);
- return new InternalValue(blob);
+ /**
+ * Get the internal value for this blob.
+ *
+ * @param identifier the identifier
+ * @param store the data store
+ * @param verify verify if the record exists, and return null if not
+ * @return the internal value or null
+ */
+ static InternalValue getInternalValue(DataIdentifier identifier, DataStore store, boolean verify) throws DataStoreException {
+ if (verify) {
+ if (store.getRecordIfStored(identifier) == null) {
+ return null;
+ }
+ } else {
+ // access the record to ensure it is not garbage collected
+ store.getRecord(identifier);
}
- return null;
+ // it exists - so we don't need to stream it again
+ // but we need to create a new object because the original
+ // one might be in a different data store (repository)
+ BLOBFileValue blob = BLOBInDataStore.getInstance(store, identifier);
+ return new InternalValue(blob);
}
/**
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/ValueFactoryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/ValueFactoryImpl.java?rev=1022094&r1=1022093&r2=1022094&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/ValueFactoryImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/ValueFactoryImpl.java Wed Oct 13 13:21:11 2010
@@ -85,8 +85,14 @@ public class ValueFactoryImpl extends Va
public Value createValue(Binary binary) {
try {
if (binary instanceof BLOBInDataStore) {
- DataIdentifier identifier = ((BLOBInDataStore) binary).getDataIdentifier();
- InternalValue value = InternalValue.getInternalValue(identifier, store);
+ BLOBInDataStore blob = (BLOBInDataStore) binary;
+ DataIdentifier identifier = blob.getDataIdentifier();
+ InternalValue value;
+ if (blob.usesDataStore(store)) {
+ value = InternalValue.getInternalValue(identifier, store, false);
+ } else {
+ value = InternalValue.getInternalValue(identifier, store, true);
+ }
if (value != null) {
// if the value is already in this data store
return new BinaryValueImpl(value.getBLOBFileValue());