Avoid inefficient reads, and allow utilization of blob buffer larger …

…than segment size on read

jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaBlob.java

@@ -28,11 +28,13 @@
 import org.firebirdsql.gds.ng.FbExceptionBuilder;
 import org.firebirdsql.gds.ng.LockCloseable;
 import org.firebirdsql.gds.ng.listeners.DatabaseListener;
+import org.firebirdsql.jdbc.SQLStateConstants;
 import org.firebirdsql.jna.fbclient.FbClientLibrary;
 import org.firebirdsql.jna.fbclient.ISC_STATUS;
 
 import java.nio.ByteBuffer;
 import java.sql.SQLException;
+import java.sql.SQLNonTransientException;
 
 import static org.firebirdsql.gds.JaybirdErrorCodes.jb_blobGetSegmentNegative;
 import static org.firebirdsql.gds.JaybirdErrorCodes.jb_blobPutSegmentEmpty;
@@ -195,16 +197,22 @@ private ByteBuffer getSegment0(int sizeRequested, ShortByReference actualLength)
     }
 
     @Override
-    public int get(final byte[] b, final int off, final int len) throws SQLException {
+    protected int get(final byte[] b, final int off, final int len, final int minLen) throws SQLException {
         try (LockCloseable ignored = withLock())  {
             validateBufferLength(b, off, len);
             if (len == 0) return 0;
+            if (minLen <= 0 || minLen > len ) {
+                throw new SQLNonTransientException(
+                        "Value out of range 0 < minLen <= %d, minLen was: %d".formatted(len, minLen),
+                        SQLStateConstants.SQL_STATE_INVALID_STRING_LENGTH);
+            }
             checkDatabaseAttached();
             checkTransactionActive();
             checkBlobOpen();
-            ShortByReference actualLength = new ShortByReference();
+
+            var actualLength = new ShortByReference();
             int count = 0;
-            while (count < len && !isEof()) {
+            while (count < minLen && !isEof()) {
                 // We honor the configured buffer size unless we somehow already allocated a bigger buffer earlier
                 ByteBuffer segmentBuffer = getSegment0(
                         Math.min(len - count, Math.max(getBlobBufferSize(), currentBufferCapacity())),

src/docs/asciidoc/release_notes.adoc

@@ -820,14 +820,29 @@ For connections to Firebird 2.1 and higherfootnote:[Formally, only Firebird 3.0
 The maximum segment size is the maximum size for sending segments (_put_) to the server.
 Due to protocol limitations, retrieving segments from the server (_get_) is two bytes (or multiples of two bytes) shorterfootnote:[For _get_ the maximum segment size is actually the maximum buffer size to receive one or more segments which are prefixed with two bytes for the length].
 
+[#blob-buffer-size]
+==== Effectiveness of `blobBufferSize` larger than maximum segment size
+
+Previously, when reading blobs, a `blobBufferSize` larger than the maximum segment size was effectively ignored.
+Now, when reading through an input stream, a `blobBufferSize` larger than the maximum segment size can be used.
+
+Jaybird will use one or more roundtrips to fill the buffer.
+To avoid inefficient fetches, a minimum of 90% of the buffer size will be filled up to the `blobBufferSize`.
+This change is not likely to improve performance, but it may allow for optimizations when reading or transferring data in large chunks.
+
+In general, setting the `blobBufferSize` larger than 65535 bytes will likely not improve performance.
+
 [#blob-put-segment-limit]
 ==== Internal API changes for `FbBlob`
 
 Two new methods were added to `FbBlob`:
 
-`int get(byte[] b, int off, int len)`:: populates the array `b`, starting at `off`, for the requested `len` bytes from the blob, and returning the actual read bytes.
+`int get(byte[] b, int off, int len)`:: populates the array `b`, starting at `off`, for the requested `len` bytes from the blob, and returns the actual number of bytes read.
 This method will read until `len` bytes have been read, and only return less than `len` when end-of-blob was reached.
 
+`int get(byte[] b, int off, int len, float minFillFactor)`:: populates the array `b`, starting at `off`, for
+ at least `minFillFactor` * `len` bytes (up to `len` bytes) from the blob, and returns the actual number of bytes read.
+
 `void put(byte[] b, int off, int len)`:: sends data from array `b` to the blob, starting at `off`, for the requested `len` bytes.
 
 The documentation of method `FbBlob.putSegment(byte[])` contradicted itself, by requiring implementations to batch larger arrays, but also requiring them to throw an exception for larger arrays, and the actual implementations provided by Jaybird threw an exception.

src/jna-test/org/firebirdsql/gds/ng/jna/JnaBlobInputTest.java

@@ -20,7 +20,7 @@
 
 import org.firebirdsql.common.FBTestProperties;
 import org.firebirdsql.common.extension.GdsTypeExtension;
-import org.firebirdsql.gds.ng.BaseTestBlob;
+import org.firebirdsql.gds.ng.BaseTestInputBlob;
 import org.firebirdsql.gds.ng.FbConnectionProperties;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -32,7 +32,7 @@
  *
  * @author Mark Rotteveel
  */
-class JnaBlobInputTest extends BaseTestBlob {
+class JnaBlobInputTest extends BaseTestInputBlob {
 
     @RegisterExtension
     @Order(1)

src/main/org/firebirdsql/gds/ng/AbstractFbBlob.java

@@ -183,6 +183,42 @@ public void putSegment(byte[] segment) throws SQLException {
         put(segment, 0, segment.length);
     }
 
+    @Override
+    public final int get(byte[] b, int off, int len) throws SQLException {
+        // requested length is minimum length
+        return get(b, off, len, len);
+    }
+
+    @Override
+    public final int get(byte[] b, int off, int len, float minFillFactor) throws SQLException {
+        if (minFillFactor <= 0f || minFillFactor > 1f || Float.isNaN(minFillFactor)) {
+            var invalidFloatFactor = new SQLNonTransientException(
+                    "minFillFactor out of range, must be 0 < minFillFactor <= 1, was: " + minFillFactor);
+            exceptionListenerDispatcher.errorOccurred(invalidFloatFactor);
+            throw invalidFloatFactor;
+        }
+        return get(b, off, len, len != 0 ? Math.max(1, (int) (minFillFactor * len)) : 0);
+    }
+
+    /**
+     * Default implementation for {@link #get(byte[], int, int)} and {@link #get(byte[], int, int, float)}.
+     *
+     * @param b
+     *         target byte array
+     * @param off
+     *         offset to start
+     * @param len
+     *         number of bytes
+     * @param minLen
+     *         minimum number of bytes to fill (must be {@code 0 < minLen <= len} if {@code len != 0}
+     * @return actual number of bytes read; is {@code 0} if {@code len == 0}, will only be less than {@code minLen} if
+     * end-of-blob is reached
+     * @throws SQLException
+     *         for database access errors, if {@code off < 0}, {@code len < 0}, or if {@code off + len > b.length},
+     *         or {@code len != 0 && (minLen <= 0 || minLen > len)}
+     */
+    protected abstract int get(byte[] b, int off, int len, int minLen) throws SQLException;
+
     /**
      * Release Java resources held. This should not communicate with the Firebird server.
      */
@@ -416,9 +452,8 @@ public int getMaximumSegmentSize() {
 
     private static int maximumSegmentSize(FbDatabase db) {
         // Max size in FB 2.1 and higher is 2^16 - 1, not 2^15 - 3 (IB 6 docs mention max is 32KiB)
-        if (db != null && (db.getOdsMajor() > 11 || db.getOdsMajor() == 11 && db.getOdsMinor() >= 1)) {
-            /* ODS 11.1 is Firebird 2.1
-               NOTE: getSegment can retrieve at most 65533 bytes of blob data as the buffer to receive segments is
+        if (db != null && db.getServerVersion().isEqualOrAbove(2, 1)) {
+            /* NOTE: getSegment can retrieve at most 65533 bytes of blob data as the buffer to receive segments is
                max 65535 bytes, but the contents of the buffer are one or more segments prefixed with 2-byte lengths;
                putSegment can write max 65535 bytes, because the buffer *is* the segment */
             return 65535;

src/main/org/firebirdsql/gds/ng/FbBlob.java

@@ -142,6 +142,13 @@ public interface FbBlob extends ExceptionListenable, AutoCloseable {
      * Contrary to similar methods like {@link java.io.InputStream#read(byte[], int, int)}, this method returns
      * {@code 0} when no bytes were read if end-of-blob is reached without reading any bytes, not {@code -1}.
      * </p>
+     * <p>
+     * Given this method attempts to fulfill the entire request for {@code len} bytes, it may not always be efficient.
+     * For example, requests near multiples of the maximum segment size (or blob buffer size) may result in a final
+     * request for just a few bytes. This is not a problem if the entire blob is requested at once, but for intermediate
+     * buffering it might be better not to do that final request, and instead work with a smaller number of bytes than
+     * requested. For those cases, use {@link #get(byte[], int, int, float)}.
+     * </p>
      *
      * @param b
      *         target byte array
@@ -156,6 +163,42 @@ public interface FbBlob extends ExceptionListenable, AutoCloseable {
      */
     int get(byte[] b, int off, int len) throws SQLException;
 
+    /**
+     * Variant of {@link #get(byte[], int, int)} to exert some control over the number of requests performed.
+     * <p>
+     * This method will request segments until at least {@code (int) (minFillFactor * len)} bytes have been retrieved,
+     * or end-of-blob is reached. This method is intended as an alternative to {@link #get(byte[], int, int)} where
+     * avoiding the potential inefficiencies of that method are preferred over getting all the requested {@code len}
+     * bytes.
+     * </p>
+     * <p>
+     * If the implementation cannot perform reads without additional allocation, it should use at most
+     * {@link DatabaseConnectionProperties#getBlobBufferSize()} as an internal buffer. If the implementation can
+     * perform reads without additional allocation, it is recommended it performs reads using (at most)
+     * {@link #getMaximumSegmentSize()}.
+     * </p>
+     * <p>
+     * Contrary to similar methods like {@link java.io.InputStream#read(byte[], int, int)}, this method returns
+     * {@code 0} when no bytes were read if end-of-blob is reached without reading any bytes, not {@code -1}.
+     * </p>
+     *
+     * @param b
+     *         target byte array
+     * @param off
+     *         offset to start
+     * @param len
+     *         number of bytes
+     * @param minFillFactor
+     *         minimum fill factor ({@code 0 < minFillFactor <= 1})
+     * @return actual number of bytes read, this method returns at least {@code (int) (minFillFactor * len)} bytes,
+     * unless end-of-blob is reached
+     * @throws SQLException
+     *         for database access errors, if {@code off < 0}, {@code len < 0}, or if {@code off + len > b.length},
+     *         {@code minFillFactor <= 0}, or {@code minFillFactor > 1} or {@code minFillFactor is NaN}
+     * @since 6
+     */
+    int get(byte[] b, int off, int len, float minFillFactor) throws SQLException;
+
     /**
      * Writes a segment of blob data.
      * <p>

src/main/org/firebirdsql/gds/ng/wire/AbstractFbWireOutputBlob.java

@@ -81,7 +81,7 @@ private void readNotSupported() throws SQLException {
     }
 
     @Override
-    public int get(byte[] b, int off, int len) throws SQLException {
+    protected final int get(byte[] b, int off, int len, int minLen) throws SQLException {
         readNotSupported();
         return -1;
     }

src/main/org/firebirdsql/gds/ng/wire/version10/V10InputBlob.java

@@ -26,9 +26,11 @@
 import org.firebirdsql.gds.ng.LockCloseable;
 import org.firebirdsql.gds.ng.listeners.DatabaseListener;
 import org.firebirdsql.gds.ng.wire.*;
+import org.firebirdsql.jdbc.SQLStateConstants;
 
 import java.io.IOException;
 import java.sql.SQLException;
+import java.sql.SQLNonTransientException;
 import java.sql.SQLWarning;
 
 import static org.firebirdsql.gds.JaybirdErrorCodes.jb_blobGetSegmentNegative;
@@ -177,29 +179,35 @@ protected void sendGetSegment(int len) throws SQLException, IOException {
     }
 
     @Override
-    public int get(final byte[] b, final int off, final int len) throws SQLException {
+    protected int get(final byte[] b, final int off, final int len, final int minLen) throws SQLException {
         try (LockCloseable ignored = withLock()) {
             validateBufferLength(b, off, len);
+            if (len == 0) return 0;
+            if (minLen <= 0 || minLen > len ) {
+                throw new SQLNonTransientException("Value out of range 0 < minLen <= len, minLen was: " + minLen,
+                        SQLStateConstants.SQL_STATE_INVALID_STRING_LENGTH);
+            }
             checkDatabaseAttached();
             checkTransactionActive();
             checkBlobOpen();
 
+            final FbWireOperations wireOps = getDatabase().getWireOperations();
+            final XdrOutputStream xdrOut = getXdrOut();
+            final XdrInputStream xdrIn = getXdrIn();
             int count = 0;
-            while (count < len && !isEof()) {
+            while (count < minLen && !isEof()) {
                 try {
                     sendGetSegment(segmentRequestSize(len - count));
-                    getXdrOut().flush();
+                    xdrOut.flush();
                 } catch (IOException e) {
                     throw FbExceptionBuilder.ioWriteError(e);
                 }
                 try {
-                    FbWireOperations wireOps = getDatabase().getWireOperations();
                     final int opCode = wireOps.readNextOperation();
                     if (opCode != op_response) {
                         wireOps.readOperationResponse(opCode, null);
                         throw new SQLException("Unexpected response to op_get_segment: " + opCode);
                     }
-                    XdrInputStream xdrIn = getXdrIn();
                     final int objHandle = xdrIn.readInt();
                     xdrIn.skipNBytes(8); // blob-id (unused)