This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 32175f205fce083591e6f8976965561ffe464fa8 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Sat Oct 30 00:39:37 2021 +0200 Avoid seek operations on very short distances (less than 8 bytes). --- .../apache/sis/internal/storage/io/ChannelDataInput.java | 15 +++++++++++++-- .../sis/internal/storage/io/ChannelDataInputTest.java | 7 ++++--- .../sis/internal/storage/io/ChannelDataOutputTest.java | 6 +++--- .../sis/internal/storage/io/ChannelDataTestCase.java | 15 ++++++++++++++- .../internal/storage/io/ChannelImageOutputStreamTest.java | 4 ++-- 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java index 3cd04e1..5f562b7 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java @@ -61,12 +61,23 @@ import static org.apache.sis.util.ArgumentChecks.ensureBetween; * {@link javax.imageio} is needed. * * @author Martin Desruisseaux (Geomatys) - * @version 1.1 + * @version 1.2 * @since 0.3 * @module */ public class ChannelDataInput extends ChannelData { /** + * Minimum number of bytes to skip in the {@code seek(long)} operation. + * If there is less bytes to skip, then it is not worth to do a seek + * and we will continue reading as normal instead. + * + * <p>This threshold is used because some formats add padding to their data for aligning on 32 bits boundary. + * It may result in very small seeks before reading the next chunk of data. A common padding is 32 bits, + * but there is no harm in using a larger one here (64 bits).</p> + */ + private static final int SEEK_THRESHOLD = Long.BYTES; + + /** * The channel from where data are read. * This is supplied at construction time. */ @@ -864,7 +875,7 @@ public class ChannelDataInput extends ChannelData { * Requested position is inside the current limits of the buffer. */ buffer.position((int) p); - } else if (channel instanceof SeekableByteChannel) { + } else if ((p < 0 || p - buffer.limit() >= SEEK_THRESHOLD) && channel instanceof SeekableByteChannel) { /* * Requested position is outside the current limits of the buffer, * but we can set the new position directly in the channel. Note diff --git a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataInputTest.java b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataInputTest.java index 97e23fc..cbf4a8a 100644 --- a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataInputTest.java +++ b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataInputTest.java @@ -32,7 +32,7 @@ import static org.junit.Assert.*; * of that buffer is used for the tests, while the original full buffer is used for comparison purpose. * * @author Martin Desruisseaux (Geomatys) - * @version 0.5 + * @version 1.2 * @since 0.3 * @module */ @@ -58,8 +58,9 @@ public final strictfp class ChannelDataInputTest extends ChannelDataTestCase { public void testAllReadMethods() throws IOException { final byte[] array = createRandomArray(STREAM_LENGTH); referenceStream = new DataInputStream(new ByteArrayInputStream(array)); - testedStream = new ChannelDataInput("testAllReadMethods", new DripByteChannel(array, random, 1, 1024), - ByteBuffer.allocate(random.nextInt(BUFFER_MAX_CAPACITY) + Double.BYTES), false); + testedStream = new ChannelDataInput("testAllReadMethods", + new DripByteChannel(array, random, 1, 1024), + ByteBuffer.allocate(randomBufferCapacity()), false); transferRandomData(testedStream, array.length - ARRAY_MAX_LENGTH, 16); } diff --git a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataOutputTest.java b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataOutputTest.java index 8366c64..cfacc0b 100644 --- a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataOutputTest.java +++ b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataOutputTest.java @@ -38,7 +38,7 @@ import static org.junit.Assert.*; * * @author Rémi Maréchal (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 0.5 + * @version 1.2 * @since 0.5 * @module */ @@ -108,7 +108,7 @@ public strictfp class ChannelDataOutputTest extends ChannelDataTestCase { */ @Test public void testAllWriteMethods() throws IOException { - initialize("testAllWriteMethods", STREAM_LENGTH, random.nextInt(BUFFER_MAX_CAPACITY) + Double.BYTES); + initialize("testAllWriteMethods", STREAM_LENGTH, randomBufferCapacity()); writeInStreams(); assertStreamContentEquals(); } @@ -132,7 +132,7 @@ public strictfp class ChannelDataOutputTest extends ChannelDataTestCase { @Test @DependsOnMethod("testAllWriteMethods") public void testWriteAndSeek() throws IOException { - initialize("testWriteAndSeek", STREAM_LENGTH, random.nextInt(BUFFER_MAX_CAPACITY) + Double.BYTES); + initialize("testWriteAndSeek", STREAM_LENGTH, randomBufferCapacity()); writeInStreams(); ((Closeable) referenceStream).close(); final byte[] expectedArray = expectedData.toByteArray(); diff --git a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataTestCase.java b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataTestCase.java index 7c7575f..3d5705f 100644 --- a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataTestCase.java +++ b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelDataTestCase.java @@ -29,7 +29,7 @@ import org.apache.sis.util.Debug; * * @author Rémi Maréchal (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 0.5 + * @version 1.2 * @since 0.5 * @module */ @@ -43,6 +43,11 @@ abstract strictfp class ChannelDataTestCase extends TestCase { static final int ARRAY_MAX_LENGTH = 256; /** + * The minimal capacity of the buffer to use for write operations. + */ + static final int BUFFER_MIN_CAPACITY = Double.BYTES; + + /** * The maximal capacity of the buffer to use for write operations. */ static final int BUFFER_MAX_CAPACITY = ARRAY_MAX_LENGTH / 4; @@ -82,6 +87,14 @@ abstract strictfp class ChannelDataTestCase extends TestCase { } /** + * Returns a random buffer capacity between {@value #BUFFER_MIN_CAPACITY} + * and {@value #BUFFER_MAX_CAPACITY} inclusive. + */ + final int randomBufferCapacity() { + return random.nextInt(BUFFER_MAX_CAPACITY - BUFFER_MIN_CAPACITY + 1) + BUFFER_MIN_CAPACITY; + } + + /** * Transfers many random data between the channel and the buffer. * This method invokes {@link #transferRandomData(int)} in a loop, * with an operation identifier selected randomly between 0 inclusive to {@code numOperations} exclusive. diff --git a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelImageOutputStreamTest.java b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelImageOutputStreamTest.java index cb76884..a78e3d4 100644 --- a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelImageOutputStreamTest.java +++ b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/ChannelImageOutputStreamTest.java @@ -33,7 +33,7 @@ import static org.junit.Assert.*; * * @author Rémi Maréchal (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 0.5 + * @version 1.2 * @since 0.5 * @module */ @@ -58,7 +58,7 @@ public final strictfp class ChannelImageOutputStreamTest extends ChannelDataOutp */ @Test public void testWriteBits() throws IOException { - initialize("testWriteBits", STREAM_LENGTH, random.nextInt(BUFFER_MAX_CAPACITY) + Long.BYTES); + initialize("testWriteBits", STREAM_LENGTH, randomBufferCapacity()); final ImageOutputStream referenceStream = (ImageOutputStream) this.referenceStream; final int length = testedStreamBackingArray.length - ARRAY_MAX_LENGTH; // Keep a margin against buffer underflow. while (testedStream.getStreamPosition() < length) {