From 4e7433dcaab57d84c796fac639f5f91d4444fb38 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Wed, 9 Aug 2023 23:54:42 -0400 Subject: [PATCH 1/3] Add CharBufferReader --- .../jackson/core/io/CharBufferReader.java | 66 +++++++++++ .../jackson/core/io/CharBufferReaderTest.java | 103 ++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 src/main/java/com/fasterxml/jackson/core/io/CharBufferReader.java create mode 100644 src/test/java/com/fasterxml/jackson/core/io/CharBufferReaderTest.java diff --git a/src/main/java/com/fasterxml/jackson/core/io/CharBufferReader.java b/src/main/java/com/fasterxml/jackson/core/io/CharBufferReader.java new file mode 100644 index 0000000000..1da5380d26 --- /dev/null +++ b/src/main/java/com/fasterxml/jackson/core/io/CharBufferReader.java @@ -0,0 +1,66 @@ +package com.fasterxml.jackson.core.io; + +import java.io.Reader; +import java.nio.CharBuffer; + +public class CharBufferReader extends Reader { + private final CharBuffer charBuffer; + + public CharBufferReader(CharBuffer buffer) { + this.charBuffer = buffer.duplicate(); + } + + @Override + public int read(char[] chars, int off, int len) { + int remaining = this.charBuffer.remaining(); + if (remaining <= 0) { + return -1; + } + int length = Math.min(len, remaining); + this.charBuffer.get(chars, off, length); + return length; + } + + @Override + public int read() { + if (this.charBuffer.hasRemaining()) { + return this.charBuffer.get(); + } + return -1; + } + + @Override + public long skip(long n) { + if (n < 0L) { + throw new IllegalArgumentException("number of characters to skip cannot be negative"); + } + int skipped = Math.min((int) n, this.charBuffer.remaining()); + this.charBuffer.position(this.charBuffer.position() + skipped); + return skipped; + } + + @Override + public boolean ready() { + return true; + } + + @Override + public boolean markSupported() { + return true; + } + + @Override + public void mark(int readAheadLimit) { + this.charBuffer.mark(); + } + + @Override + public void reset() { + this.charBuffer.reset(); + } + + @Override + public void close() { + this.charBuffer.position(this.charBuffer.limit()); + } +} \ No newline at end of file diff --git a/src/test/java/com/fasterxml/jackson/core/io/CharBufferReaderTest.java b/src/test/java/com/fasterxml/jackson/core/io/CharBufferReaderTest.java new file mode 100644 index 0000000000..eec1709f07 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/core/io/CharBufferReaderTest.java @@ -0,0 +1,103 @@ +package com.fasterxml.jackson.core.io; + +import java.io.IOException; +import java.io.Reader; +import java.nio.CharBuffer; +import java.util.Arrays; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertThrows; + +public class CharBufferReaderTest extends com.fasterxml.jackson.core.BaseTest { + + public void testSingleCharRead() throws IOException { + CharBuffer buffer = CharBuffer.wrap("aB\u00A0\u1AE9\uFFFC"); + Reader charBufferReader = new CharBufferReader(buffer); + try (Reader reader = charBufferReader) { + assertEquals('a', reader.read()); + assertEquals('B', reader.read()); + assertEquals('\u00A0', reader.read()); + assertEquals('\u1AE9', reader.read()); + assertEquals('', reader.read()); + assertEquals(-1, reader.read()); + } + assertEquals(-1, charBufferReader.read()); + } + + + public void testBulkRead() throws IOException { + CharBuffer buffer = CharBuffer.wrap("abcdefghijklmnopqrst\u00A0"); + char[] chars = new char[12]; + Reader charBufferReader = new CharBufferReader(buffer); + try (Reader reader = charBufferReader) { + assertEquals(12, reader.read(chars)); + assertArrayEquals("abcdefghijkl".toCharArray(), chars); + assertEquals(9, reader.read(chars)); + assertArrayEquals("mnopqrst\u00A0".toCharArray(), Arrays.copyOf(chars, 9)); + assertEquals(-1, reader.read(chars)); + } + assertEquals(-1, charBufferReader.read(chars)); + } + + + public void testSkip() throws IOException { + CharBuffer buffer = CharBuffer.wrap("abcdefghijklmnopqrst\u00A0"); + Reader charBufferReader = new CharBufferReader(buffer); + char[] chars = new char[12]; + try (Reader reader = charBufferReader) { + assertEquals(12, reader.read(chars)); + assertArrayEquals("abcdefghijkl".toCharArray(), chars); + assertEquals(4, reader.skip(4)); + assertEquals(5, reader.read(chars, 3, 9)); + assertArrayEquals("qrst\u00A0".toCharArray(), Arrays.copyOfRange(chars, 3, 8)); + assertEquals(0, reader.skip(0)); + assertEquals(0, reader.skip(1)); + } + assertEquals(0, charBufferReader.skip(1)); + } + + + public void testInvalidSkip() throws IOException { + try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> reader.skip(-1)); + assertEquals("number of characters to skip cannot be negative", exception.getMessage()); + } + } + + + public void testReady() throws IOException { + try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { + assertEquals(true, reader.ready()); + } + } + + + public void testMarkReset() throws IOException { + char[] chars = new char[8]; + try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { + assertEquals(true, reader.markSupported()); + reader.mark(3); + assertEquals(3, reader.read(chars, 0, 3)); + assertArrayEquals("tes".toCharArray(), Arrays.copyOf(chars, 3)); + reader.reset(); + assertEquals(4, reader.read(chars)); + assertArrayEquals("test".toCharArray(), Arrays.copyOf(chars, 4)); + reader.reset(); + Arrays.fill(chars, '\0'); + assertEquals(4, reader.read(chars)); + } + } + + + public void testClose() throws IOException { + char[] chars = new char[2]; + try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { + assertEquals(2, reader.read(chars)); + assertArrayEquals("te".toCharArray(), chars); + reader.close(); + Arrays.fill(chars, '\0'); + assertEquals(-1, reader.read(chars)); + assertArrayEquals("\0\0".toCharArray(), chars); + } + } +} \ No newline at end of file From df0994364f98de82d0ff2c2f49e6bf9589d71451 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Wed, 9 Aug 2023 23:55:23 -0400 Subject: [PATCH 2/3] ByteSourceJsonBootstrapper uses CharBufferReader for byte[] inputs --- .../jackson/core/json/ByteSourceJsonBootstrapper.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java b/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java index b2b9d088bf..e48c6142ae 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java @@ -1,6 +1,9 @@ package com.fasterxml.jackson.core.json; import java.io.*; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.Charset; import com.fasterxml.jackson.core.*; import com.fasterxml.jackson.core.format.InputAccessor; @@ -230,7 +233,9 @@ public Reader constructReader() throws IOException InputStream in = _in; if (in == null) { - in = new ByteArrayInputStream(_inputBuffer, _inputPtr, _inputEnd); + // [jackson-core#488] Avoid overhead of InputStreamReader allocating ByteBuffer + ByteBuffer byteBuffer = ByteBuffer.wrap(_inputBuffer, _inputPtr, _inputEnd); + return new CharBufferReader(Charset.forName(enc.getJavaName()).decode(byteBuffer)); } else { /* Also, if we have any read but unused input (usually true), * need to merge that input in: From f63bcd55e0d0fe4edc32d0f3378ed39f57a0772d Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 10 Aug 2023 12:33:14 -0400 Subject: [PATCH 3/3] Encapsulate CharBufferReader implementation Address PR comments --- .../core/json/ByteSourceJsonBootstrapper.java | 12 ++++++---- .../core/{io => json}/CharBufferReader.java | 21 ++++++++++++----- .../{io => json}/CharBufferReaderTest.java | 23 +++++++++++-------- 3 files changed, 37 insertions(+), 19 deletions(-) rename src/main/java/com/fasterxml/jackson/core/{io => json}/CharBufferReader.java (63%) rename src/test/java/com/fasterxml/jackson/core/{io => json}/CharBufferReaderTest.java (83%) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java b/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java index e48c6142ae..863351f82f 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ByteSourceJsonBootstrapper.java @@ -2,7 +2,6 @@ import java.io.*; import java.nio.ByteBuffer; -import java.nio.CharBuffer; import java.nio.charset.Charset; import com.fasterxml.jackson.core.*; @@ -233,9 +232,14 @@ public Reader constructReader() throws IOException InputStream in = _in; if (in == null) { - // [jackson-core#488] Avoid overhead of InputStreamReader allocating ByteBuffer - ByteBuffer byteBuffer = ByteBuffer.wrap(_inputBuffer, _inputPtr, _inputEnd); - return new CharBufferReader(Charset.forName(enc.getJavaName()).decode(byteBuffer)); + int size = _inputEnd - _inputPtr; + if (size >= 0 && size <= 8192) { + // [jackson-core#488] Avoid overhead of heap ByteBuffer allocated by InputStreamReader + // when processing small inputs up to 8KiB. + Charset charset = Charset.forName(enc.getJavaName()); + return new CharBufferReader(charset.decode(ByteBuffer.wrap(_inputBuffer, _inputPtr, _inputEnd))); + } + in = new ByteArrayInputStream(_inputBuffer, _inputPtr, _inputEnd); } else { /* Also, if we have any read but unused input (usually true), * need to merge that input in: diff --git a/src/main/java/com/fasterxml/jackson/core/io/CharBufferReader.java b/src/main/java/com/fasterxml/jackson/core/json/CharBufferReader.java similarity index 63% rename from src/main/java/com/fasterxml/jackson/core/io/CharBufferReader.java rename to src/main/java/com/fasterxml/jackson/core/json/CharBufferReader.java index 1da5380d26..5fddd9eee6 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/CharBufferReader.java +++ b/src/main/java/com/fasterxml/jackson/core/json/CharBufferReader.java @@ -1,13 +1,19 @@ -package com.fasterxml.jackson.core.io; +package com.fasterxml.jackson.core.json; import java.io.Reader; import java.nio.CharBuffer; +import java.nio.charset.Charset; -public class CharBufferReader extends Reader { +/** + * An adapter implementation from {@link CharBuffer} to {@link Reader}. + * This is used by {@link ByteSourceJsonBootstrapper#constructReader()} when processing small inputs to + * avoid the 8KiB {@link java.nio.ByteBuffer} allocated by {@link java.io.InputStreamReader}, see [jackson-core#488]. + */ +final class CharBufferReader extends Reader { private final CharBuffer charBuffer; - public CharBufferReader(CharBuffer buffer) { - this.charBuffer = buffer.duplicate(); + CharBufferReader(CharBuffer buffer) { + this.charBuffer = buffer; } @Override @@ -34,7 +40,7 @@ public long skip(long n) { if (n < 0L) { throw new IllegalArgumentException("number of characters to skip cannot be negative"); } - int skipped = Math.min((int) n, this.charBuffer.remaining()); + int skipped = Math.min(this.charBuffer.remaining(), (int) Math.min(n, Integer.MAX_VALUE)); this.charBuffer.position(this.charBuffer.position() + skipped); return skipped; } @@ -51,6 +57,9 @@ public boolean markSupported() { @Override public void mark(int readAheadLimit) { + if (readAheadLimit < 0L) { + throw new IllegalArgumentException("read ahead limit cannot be negative"); + } this.charBuffer.mark(); } @@ -63,4 +72,4 @@ public void reset() { public void close() { this.charBuffer.position(this.charBuffer.limit()); } -} \ No newline at end of file +} diff --git a/src/test/java/com/fasterxml/jackson/core/io/CharBufferReaderTest.java b/src/test/java/com/fasterxml/jackson/core/json/CharBufferReaderTest.java similarity index 83% rename from src/test/java/com/fasterxml/jackson/core/io/CharBufferReaderTest.java rename to src/test/java/com/fasterxml/jackson/core/json/CharBufferReaderTest.java index eec1709f07..17736ca6e5 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/CharBufferReaderTest.java +++ b/src/test/java/com/fasterxml/jackson/core/json/CharBufferReaderTest.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.core.io; +package com.fasterxml.jackson.core.json; import java.io.IOException; import java.io.Reader; @@ -24,7 +24,6 @@ public void testSingleCharRead() throws IOException { assertEquals(-1, charBufferReader.read()); } - public void testBulkRead() throws IOException { CharBuffer buffer = CharBuffer.wrap("abcdefghijklmnopqrst\u00A0"); char[] chars = new char[12]; @@ -39,7 +38,6 @@ public void testBulkRead() throws IOException { assertEquals(-1, charBufferReader.read(chars)); } - public void testSkip() throws IOException { CharBuffer buffer = CharBuffer.wrap("abcdefghijklmnopqrst\u00A0"); Reader charBufferReader = new CharBufferReader(buffer); @@ -48,15 +46,17 @@ public void testSkip() throws IOException { assertEquals(12, reader.read(chars)); assertArrayEquals("abcdefghijkl".toCharArray(), chars); assertEquals(4, reader.skip(4)); - assertEquals(5, reader.read(chars, 3, 9)); - assertArrayEquals("qrst\u00A0".toCharArray(), Arrays.copyOfRange(chars, 3, 8)); + assertEquals(4, reader.read(chars, 3, 4)); + assertArrayEquals("qrst".toCharArray(), Arrays.copyOfRange(chars, 3, 7)); + assertEquals(1, reader.skip(Long.MAX_VALUE)); + assertEquals(0, reader.skip(Integer.MAX_VALUE)); + assertEquals(0, reader.skip(Long.MAX_VALUE)); assertEquals(0, reader.skip(0)); assertEquals(0, reader.skip(1)); } assertEquals(0, charBufferReader.skip(1)); } - public void testInvalidSkip() throws IOException { try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> reader.skip(-1)); @@ -64,14 +64,12 @@ public void testInvalidSkip() throws IOException { } } - public void testReady() throws IOException { try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { assertEquals(true, reader.ready()); } } - public void testMarkReset() throws IOException { char[] chars = new char[8]; try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { @@ -80,6 +78,7 @@ public void testMarkReset() throws IOException { assertEquals(3, reader.read(chars, 0, 3)); assertArrayEquals("tes".toCharArray(), Arrays.copyOf(chars, 3)); reader.reset(); + reader.mark(Integer.MAX_VALUE); assertEquals(4, reader.read(chars)); assertArrayEquals("test".toCharArray(), Arrays.copyOf(chars, 4)); reader.reset(); @@ -88,6 +87,12 @@ public void testMarkReset() throws IOException { } } + public void testInvalidMark() throws IOException { + try (Reader reader = new CharBufferReader(CharBuffer.wrap("test"))) { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> reader.mark(-1)); + assertEquals("read ahead limit cannot be negative", exception.getMessage()); + } + } public void testClose() throws IOException { char[] chars = new char[2]; @@ -100,4 +105,4 @@ public void testClose() throws IOException { assertArrayEquals("\0\0".toCharArray(), chars); } } -} \ No newline at end of file +}