jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From thom...@apache.org
Subject svn commit: r1022094 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: data/ value/
Date Wed, 13 Oct 2010 13:21:11 GMT
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());



Mime
View raw message