db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From krist...@apache.org
Subject svn commit: r726683 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/jdbc/TemporaryClob.java testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ClobTest.java
Date Mon, 15 Dec 2008 11:37:41 GMT
Author: kristwaa
Date: Mon Dec 15 03:37:40 2008
New Revision: 726683

URL: http://svn.apache.org/viewvc?rev=726683&view=rev
Log:
DERBY-3934: Improve performance of reading modified Clobs.
There are two major changes with this patch:
 a) The implementation of getInternalReader improves the performance of
    getSubString significantly. This benefits the embedded driver too, but it
    is crucial for the performance of all read operations on Clob from the
    client driver.
    Again, the mechanism used to get better performance is to keep an internal 
    reader around to avoid repositioning on every request.

 b) Added caching and updating of the Clob character length.

Also added some tests for verifying that the Clob length is handled correctly.
Patch file: derby-3934-4a-getinternalreader_cachedlength.diff


Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/TemporaryClob.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ClobTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/TemporaryClob.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/TemporaryClob.java?rev=726683&r1=726682&r2=726683&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/TemporaryClob.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/TemporaryClob.java Mon Dec
15 03:37:40 2008
@@ -24,6 +24,7 @@
 
 import java.io.BufferedInputStream;
 import java.io.EOFException;
+import java.io.FilterReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
@@ -31,6 +32,7 @@
 import java.sql.SQLException;
 import org.apache.derby.iapi.error.StandardException;
 import org.apache.derby.iapi.jdbc.CharacterStreamDescriptor;
+import org.apache.derby.iapi.types.TypeId;
 import org.apache.derby.iapi.util.UTF8Util;
 
 /**
@@ -54,6 +56,26 @@
     /** Tells whether this Clob has been released or not. */
     // GuardedBy("this")
     private boolean released = false;
+    /**
+     * Cached character length of the Clob.
+     * <p>
+     * A value of {@code 0} is interpreted as unknown length, even though it is
+     * a valid value. If the length is requested and the value is zero, an
+     * attempt to obtain the length is made by draining the source.
+     */
+    private long cachedCharLength;
+    /**
+     * Shared internal reader, closed when the Clob is released.
+     * This is a performance optimization, and the stream is shared between
+     * "one time" operations, for instance {@code getSubString} calls. Often a
+     * subset, or the whole, of the Clob is read subsequently and then this
+     * optimization avoids repositioning costs (the store does not support
+     * random access for LOBs).
+     * <b>NOTE</b>: Do not publish this reader to the end-user.
+     */
+    private UTF8Reader internalReader;
+    /** The internal reader wrapped so that it cannot be closed. */
+    private FilterReader unclosableInternalReader;
 
     /** Simple one-entry cache for character-byte position. */
     // @GuardedBy("this")
@@ -126,6 +148,11 @@
         if (!this.released) {
             this.released = true;
             this.bytes.free();
+            if (internalReader != null) {
+                internalReader.close();
+                internalReader = null;
+                unclosableInternalReader = null;
+            }
         }
     }
 
@@ -241,15 +268,10 @@
             throw new IllegalArgumentException(
                 "Position must be positive: " + pos);
         }
-        // Describe the stream to allow the reader to configure itself.
-        CharacterStreamDescriptor csd = new CharacterStreamDescriptor.Builder().
-                stream(this.bytes.getInputStream(0)).
-                positionAware(true).
-                bufferable(this.bytes.getLength() > 4096). // Cache if on disk.
-                byteLength(this.bytes.getLength()).
-                build();
-        Reader isr = new UTF8Reader(
-                csd, conChild, conChild.getConnectionSynchronization());
+        // getCSD obtains a descriptor for the stream to allow the reader
+        // to configure itself.
+        Reader isr = new UTF8Reader(getCSD(), conChild,
+                conChild.getConnectionSynchronization());
 
         long leftToSkip = pos -1;
         long skipped;
@@ -270,8 +292,25 @@
      */
     public Reader getInternalReader(long characterPosition)
             throws IOException, SQLException {
-        // TODO: See if we can optimize for a shared internal reader.
-        return getReader(characterPosition);
+        if (this.internalReader == null) {
+            // getCSD obtains a descriptor for the stream to allow the reader
+            // to configure itself.
+            this.internalReader = new UTF8Reader(getCSD(), conChild,
+                    conChild.getConnectionSynchronization());
+            this.unclosableInternalReader =
+                    new FilterReader(this.internalReader) {
+                        public void close() {
+                            // Do nothing.
+                            // Stream will be closed when the Clob is released.
+                        }
+                    };
+        }
+        try {
+            this.internalReader.reposition(characterPosition);
+        } catch (StandardException se) {
+            throw Util.generateCsSQLException(se);
+        }
+        return this.unclosableInternalReader;
     }
 
     /**
@@ -282,8 +321,11 @@
      */
     public synchronized long getCharLength() throws IOException {
         checkIfValid();
-        return
-            UTF8Util.skipUntilEOF(new BufferedInputStream(getRawByteStream()));
+        if (cachedCharLength == 0) {
+            cachedCharLength = UTF8Util.skipUntilEOF(
+                    new BufferedInputStream(getRawByteStream()));
+        }
+        return cachedCharLength;
     }
 
     /**
@@ -314,6 +356,8 @@
             throw new IllegalArgumentException(
                 "Position must be positive: " + insertionPoint);
         }
+        long prevLength = cachedCharLength;
+        updateInternalState(insertionPoint);
         long byteInsertionPoint = getBytePosition(insertionPoint);
         long curByteLength = this.bytes.getLength();
         byte[] newBytes = getByteFromString(str);
@@ -344,6 +388,16 @@
             } catch (StandardException se) {
                 throw Util.generateCsSQLException(se);
             }
+            // Update the length if we know the previous length.
+            if (prevLength != 0) {
+                long newLength = (insertionPoint -1) + str.length();
+                if (newLength > prevLength) {
+                    cachedCharLength = newLength; // The Clob grew.
+                } else {
+                    // We only wrote over existing characters, length unchanged.
+                    cachedCharLength = prevLength;
+                }
+            }
         }
         return str.length();
     }
@@ -353,7 +407,7 @@
      *
      * @return {@code true} if released, {@code false} if not.
      */
-    public boolean isReleased() {
+    public synchronized boolean isReleased() {
         return released;
     }
 
@@ -380,10 +434,9 @@
             long byteLength = UTF8Util.skipFully (
                     new BufferedInputStream(getRawByteStream()), newCharLength);
             this.bytes.truncate(byteLength);
-            if (newCharLength <= this.posCache.getCharPos()) {
-                // Reset the cache if last cached position has been cut away.
-                this.posCache.reset();
-            }
+            // Reset the internal state, and then update the length.
+            updateInternalState(newCharLength);
+            cachedCharLength = newCharLength;
         } catch (StandardException se) {
             throw Util.generateCsSQLException(se);
         }
@@ -474,6 +527,56 @@
     }
 
     /**
+     * Updates the internal state after a modification has been performed on
+     * the Clob content.
+     * <p>
+     * Currently the state update consists of dicarding the internal reader to
+     * stop it from delivering stale data, to reset the byte/char position
+     * cache if necessary, and to reset the cached length.
+     *
+     * @param charChangePosition the position where the Clob change started
+     */
+    private final void updateInternalState(long charChangePosition) {
+        // Discard the internal reader, don't want to deliver stale data.
+        if (internalReader != null) {
+            internalReader.close();
+            internalReader = null;
+            unclosableInternalReader = null;
+        }
+        // Reset the cache if last cached position has been cut away.
+        if (charChangePosition < posCache.getCharPos()) {
+            posCache.reset();
+        }
+        // Reset the cached length.
+        cachedCharLength = 0;
+    }
+
+    /**
+     * Returns a character stream descriptor for the stream.
+     * <p>
+     * All streams from the underlying source ({@code LOBStreamControl}) are
+     * position aware and can be moved to a specific byte position cheaply.
+     * The maximum length is not really needed, nor known, at the moment, so
+     * the maximum allowed Clob length in Derby is used.
+     *
+     * @return A character stream descriptor.
+     * @throws IOException if obtaining the length of the stream fails
+     */
+    private final CharacterStreamDescriptor getCSD()
+            throws IOException {
+        return new CharacterStreamDescriptor.Builder().
+                    // Static values.
+                    positionAware(true).
+                    maxCharLength(TypeId.CLOB_MAXWIDTH).
+                    // Values depending on content and/or state.
+                    stream(bytes.getInputStream(0)).
+                    bufferable(bytes.getLength() > 4096).
+                    byteLength(bytes.getLength()).
+                    charLength(cachedCharLength). // 0 means unknown
+                    build();
+    }
+
+    /**
      * A simple class to hold the byte position for a character position.
      * <p>
      * The implementation is very simple and is basically intended to speed up

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ClobTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ClobTest.java?rev=726683&r1=726682&r2=726683&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ClobTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ClobTest.java
Mon Dec 15 03:37:40 2008
@@ -78,8 +78,6 @@
             "\u00e6\u00f8\u00e5\u00e6\u00f8\u00e5\u00e6\u00f8\u00e5" +
             "\u00e6\u00f8\u00e5\u00e6\u00f8\u00e5\u00e6\u00f8\u00e5";
 
-    /** Result set used to obtain Clob. */
-    private ResultSet rs = null;
     /**
      * The Clob used for testing.
      * It is reinitialized to a Clob containing the empty string for each test.
@@ -206,6 +204,49 @@
             newContent, this.clob.getSubString(1, newContent.length()));
     }
 
+    /**
+     * Tests that the length is updated correctly when inserting data.
+     */
+    public void testLengthAfterInsertOnEmpty()
+            throws IOException, SQLException {
+        insertDataWithToken("", 0, 0, SET_STRING);
+        assertEquals(0L, clob.length());
+        clob.setString(1, "TEST");
+        assertEquals(4L, clob.length());
+        clob.setString(1, "TEST");
+        assertEquals(4L, clob.length());
+        clob.setString(5, "TEST");
+        assertEquals(8L, clob.length());
+        clob.setString(7, "TEST");
+        assertEquals(10L, clob.length());
+        clob.truncate(4L);
+        assertEquals(4L, clob.length());
+        clob.setString(4, "TEST");
+        assertEquals(7L, clob.length());
+    }
+
+    /**
+     * Tests that the length is updated correctly when inserting data.
+     */
+    public void testLengthAfterInsertOnLarge()
+            throws IOException, SQLException {
+        final String token = "SWEETSPOT";
+        long curLength = (32+9) * 1024 + token.length();
+        insertDataWithToken(token, 32*1024, 9*1024, SET_CHARACTER_STREAM);
+        assertEquals(curLength, clob.length());
+        clob.setString(1, "TEST");
+        assertEquals(curLength, clob.length());
+        clob.setString(curLength, "X");
+        assertEquals(curLength, clob.length());
+        assertEquals(32*1024+1, clob.position(token, 17*1024));
+        clob.setString(32*1024+1, "FUNNYSPOT");
+        assertEquals(curLength, clob.length());
+        assertEquals(-1, clob.position(token, 17*1024));
+        clob.setString(curLength +1, "TEST");
+        curLength += 4;
+        assertEquals(curLength, clob.length());
+    }
+
     public void testReplaceMultibyteWithSingleByteForwards()
             throws IOException, SQLException {
         // Add some content to work on first.
@@ -480,19 +521,19 @@
         // Obtain a Clob containing the empty string ("").
         Statement stmt = createStatement();
         // Keep reference to the result set to be able to close it.
-        this.rs = stmt.executeQuery(
+        ResultSet rs = stmt.executeQuery(
                 "select dClob from ClobTestData where id = 1");
-        assertTrue(this.rs.next());
-        this.clob = this.rs.getClob(1);
+        assertTrue(rs.next());
+        this.clob = rs.getClob(1);
+        // Leave the result set open to keep the Clob alive.
     }
 
     /**
-     * Nullify reference to Clob, close the parent result set.
+     * Nullify reference to Clob.
      */
     protected void tearDown()
             throws Exception {
         this.clob = null;
-        this.rs.close();
         super.tearDown();
     }
 



Mime
View raw message