From 3619d67fd28cab72eaeae41290787ae918ca2945 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Wed, 25 Jul 2018 11:32:32 -0700 Subject: [PATCH 1/9] Update to Airbase 88 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index fbf59fa6..24aaf1ae 100644 --- a/pom.xml +++ b/pom.xml @@ -13,7 +13,7 @@ io.airlift airbase - 83 + 88 2012 From 6b71014f1fbccbd3ebefedeaccee8348b4aa1c32 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Sun, 30 Dec 2018 22:06:41 -0800 Subject: [PATCH 2/9] Update JaCoCo and SpotBugs plugins for JDK 11 --- pom.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pom.xml b/pom.xml index 24aaf1ae..7f9df4cd 100644 --- a/pom.xml +++ b/pom.xml @@ -78,6 +78,23 @@ + + + + + org.jacoco + jacoco-maven-plugin + 0.8.2 + + + + com.github.spotbugs + spotbugs-maven-plugin + 3.1.10 + + + + com.mycila From 449d76be3f7d104bc372e005c95f3261f99033b5 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Wed, 25 Jul 2018 10:55:40 -0700 Subject: [PATCH 3/9] Add Travis config for newer JDKs --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index ca046449..5342b468 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,3 +5,6 @@ dist: trusty jdk: - oraclejdk8 - oraclejdk9 + - openjdk10 + - openjdk11 + - openjdk12 From a9971a85bd8834bd9bb99873efc0914d83e760d1 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Sun, 30 Dec 2018 22:45:53 -0800 Subject: [PATCH 4/9] Catch more specific exception type --- src/main/java/io/airlift/slice/JvmUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/airlift/slice/JvmUtils.java b/src/main/java/io/airlift/slice/JvmUtils.java index e57234b1..17fbefcc 100644 --- a/src/main/java/io/airlift/slice/JvmUtils.java +++ b/src/main/java/io/airlift/slice/JvmUtils.java @@ -67,7 +67,7 @@ final class JvmUtils ADDRESS_ACCESSOR = Buffer.class.getDeclaredField("address"); ADDRESS_ACCESSOR.setAccessible(true); } - catch (Exception e) { + catch (ReflectiveOperationException e) { throw new RuntimeException(e); } } From 849f7d80ad69b723a43d7fa64daba7e656b785b6 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Sun, 30 Dec 2018 21:06:22 -0800 Subject: [PATCH 5/9] Simplify string conversion for direct memory --- src/main/java/io/airlift/slice/Slice.java | 4 +- .../java/io/airlift/slice/StringDecoder.java | 84 ------------------- 2 files changed, 1 insertion(+), 87 deletions(-) delete mode 100644 src/main/java/io/airlift/slice/StringDecoder.java diff --git a/src/main/java/io/airlift/slice/Slice.java b/src/main/java/io/airlift/slice/Slice.java index d9da9f1f..284c06d8 100644 --- a/src/main/java/io/airlift/slice/Slice.java +++ b/src/main/java/io/airlift/slice/Slice.java @@ -41,7 +41,6 @@ import static io.airlift.slice.SizeOf.sizeOfIntArray; import static io.airlift.slice.SizeOf.sizeOfLongArray; import static io.airlift.slice.SizeOf.sizeOfShortArray; -import static io.airlift.slice.StringDecoder.decodeString; import static java.lang.Math.min; import static java.lang.Math.multiplyExact; import static java.lang.String.format; @@ -1194,8 +1193,7 @@ public String toString(int index, int length, Charset charset) if (base instanceof byte[]) { return new String((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length, charset); } - // direct memory can only be converted to a string using a ByteBuffer - return decodeString(toByteBuffer(index, length), charset); + return new String(getBytes(index, length), charset); } public ByteBuffer toByteBuffer() diff --git a/src/main/java/io/airlift/slice/StringDecoder.java b/src/main/java/io/airlift/slice/StringDecoder.java deleted file mode 100644 index e58d97f0..00000000 --- a/src/main/java/io/airlift/slice/StringDecoder.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.airlift.slice; - -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.Charset; -import java.nio.charset.CharsetDecoder; -import java.nio.charset.CoderResult; -import java.nio.charset.CodingErrorAction; -import java.util.IdentityHashMap; -import java.util.Map; - -import static java.util.Objects.requireNonNull; - -final class StringDecoder -{ - private static final ThreadLocal> decoders = new ThreadLocal>() - { - @Override - protected Map initialValue() - { - return new IdentityHashMap<>(); - } - }; - - private StringDecoder() {} - - @SuppressWarnings("ObjectToString") - public static String decodeString(ByteBuffer src, Charset charset) - { - CharsetDecoder decoder = getDecoder(charset); - CharBuffer dst = CharBuffer.allocate((int) ((double) src.remaining() * decoder.maxCharsPerByte())); - try { - CoderResult cr = decoder.decode(src, dst, true); - if (!cr.isUnderflow()) { - cr.throwException(); - } - cr = decoder.flush(dst); - if (!cr.isUnderflow()) { - cr.throwException(); - } - } - catch (CharacterCodingException x) { - throw new IllegalStateException(x); - } - return dst.flip().toString(); - } - - /** - * Returns a cached thread-local {@link java.nio.charset.CharsetDecoder} for the specified charset. - */ - private static CharsetDecoder getDecoder(Charset charset) - { - requireNonNull(charset, "charset is null"); - - Map map = decoders.get(); - CharsetDecoder d = map.get(charset); - if (d != null) { - d.reset(); - d.onMalformedInput(CodingErrorAction.REPLACE); - d.onUnmappableCharacter(CodingErrorAction.REPLACE); - return d; - } - - d = charset.newDecoder(); - d.onMalformedInput(CodingErrorAction.REPLACE); - d.onUnmappableCharacter(CodingErrorAction.REPLACE); - map.put(charset, d); - return d; - } -} From 6abaa5e6f88dc984c7508e8067763088600231fb Mon Sep 17 00:00:00 2001 From: David Phillips Date: Sun, 30 Dec 2018 21:31:50 -0800 Subject: [PATCH 6/9] Add test for toByteBuffer --- src/test/java/io/airlift/slice/TestSlice.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/test/java/io/airlift/slice/TestSlice.java b/src/test/java/io/airlift/slice/TestSlice.java index a21c52a4..71ce5d83 100644 --- a/src/test/java/io/airlift/slice/TestSlice.java +++ b/src/test/java/io/airlift/slice/TestSlice.java @@ -19,6 +19,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.ByteBuffer; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -823,6 +824,38 @@ public static void assertIndexOf(Slice data, Slice pattern) assertEquals(bruteForce, indexOf); } + @Test + public void testToByteBuffer() + { + byte[] original = "hello world".getBytes(UTF_8); + + Slice slice = allocate(original.length); + slice.setBytes(0, original); + assertEquals(slice.getBytes(), original); + + assertEquals(getBytes(slice.toByteBuffer()), original); + + assertToByteBuffer(slice, original); + } + + private static void assertToByteBuffer(Slice slice, byte[] original) + { + for (int index = 0; index < original.length; index++) { + for (int length = 0; length < (original.length - index); length++) { + byte[] actual = getBytes(slice.toByteBuffer(index, length)); + byte[] expected = Arrays.copyOfRange(original, index, index + length); + assertEquals(actual, expected); + } + } + } + + private static byte[] getBytes(ByteBuffer buffer) + { + byte[] data = new byte[buffer.remaining()]; + buffer.get(data); + return data; + } + private static List createRandomLongs(int count) { Random random = new Random(); From b2e75e2a40d078b247201e34981c7c55529c2f3d Mon Sep 17 00:00:00 2001 From: David Phillips Date: Sun, 30 Dec 2018 21:56:17 -0800 Subject: [PATCH 7/9] Return zero positioned heap byte buffers --- src/main/java/io/airlift/slice/Slice.java | 2 +- src/test/java/io/airlift/slice/TestSlice.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/airlift/slice/Slice.java b/src/main/java/io/airlift/slice/Slice.java index 284c06d8..c47703ed 100644 --- a/src/main/java/io/airlift/slice/Slice.java +++ b/src/main/java/io/airlift/slice/Slice.java @@ -1206,7 +1206,7 @@ public ByteBuffer toByteBuffer(int index, int length) checkIndexLength(index, length); if (base instanceof byte[]) { - return ByteBuffer.wrap((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length); + return ByteBuffer.wrap((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length).slice(); } try { diff --git a/src/test/java/io/airlift/slice/TestSlice.java b/src/test/java/io/airlift/slice/TestSlice.java index 71ce5d83..6fb71665 100644 --- a/src/test/java/io/airlift/slice/TestSlice.java +++ b/src/test/java/io/airlift/slice/TestSlice.java @@ -851,6 +851,7 @@ private static void assertToByteBuffer(Slice slice, byte[] original) private static byte[] getBytes(ByteBuffer buffer) { + assertEquals(buffer.position(), 0); byte[] data = new byte[buffer.remaining()]; buffer.get(data); return data; From a5ec474104b8382b2e9f953fa6309a957cca8318 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Sun, 30 Dec 2018 22:42:56 -0800 Subject: [PATCH 8/9] Access direct buffer address field using Unsafe This avoids illegal access warnings on JDK 9. --- src/main/java/io/airlift/slice/JvmUtils.java | 17 +++++++---------- src/main/java/io/airlift/slice/Slices.java | 4 ++-- .../airlift/slice/TestUnsafeSliceFactory.java | 4 ++-- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/main/java/io/airlift/slice/JvmUtils.java b/src/main/java/io/airlift/slice/JvmUtils.java index 17fbefcc..db2ee346 100644 --- a/src/main/java/io/airlift/slice/JvmUtils.java +++ b/src/main/java/io/airlift/slice/JvmUtils.java @@ -23,6 +23,7 @@ import java.nio.Buffer; import java.nio.ByteBuffer; +import static io.airlift.slice.Preconditions.checkArgument; import static sun.misc.Unsafe.ARRAY_BOOLEAN_INDEX_SCALE; import static sun.misc.Unsafe.ARRAY_BYTE_INDEX_SCALE; import static sun.misc.Unsafe.ARRAY_DOUBLE_INDEX_SCALE; @@ -36,7 +37,7 @@ final class JvmUtils static final Unsafe unsafe; static final MethodHandle newByteBuffer; - private static final Field ADDRESS_ACCESSOR; + private static final long ADDRESS_OFFSET; static { try { @@ -64,8 +65,8 @@ final class JvmUtils newByteBuffer = MethodHandles.lookup().unreflectConstructor(constructor) .asType(MethodType.methodType(ByteBuffer.class, long.class, int.class, Object.class)); - ADDRESS_ACCESSOR = Buffer.class.getDeclaredField("address"); - ADDRESS_ACCESSOR.setAccessible(true); + // fetch the address field for direct buffers + ADDRESS_OFFSET = unsafe.objectFieldOffset(Buffer.class.getDeclaredField("address")); } catch (ReflectiveOperationException e) { throw new RuntimeException(e); @@ -79,14 +80,10 @@ private static void assertArrayIndexScale(final String name, int actualIndexScal } } - public static long getAddress(Buffer buffer) + static long bufferAddress(Buffer buffer) { - try { - return (long) ADDRESS_ACCESSOR.get(buffer); - } - catch (IllegalAccessException e) { - throw new RuntimeException(e); - } + checkArgument(buffer.isDirect(), "buffer is not direct"); + return unsafe.getLong(buffer, ADDRESS_OFFSET); } private JvmUtils() {} diff --git a/src/main/java/io/airlift/slice/Slices.java b/src/main/java/io/airlift/slice/Slices.java index c325bc0c..59a49ae6 100644 --- a/src/main/java/io/airlift/slice/Slices.java +++ b/src/main/java/io/airlift/slice/Slices.java @@ -23,7 +23,7 @@ import java.nio.channels.FileChannel.MapMode; import java.nio.charset.Charset; -import static io.airlift.slice.JvmUtils.getAddress; +import static io.airlift.slice.JvmUtils.bufferAddress; import static io.airlift.slice.Preconditions.checkPositionIndexes; import static java.lang.String.format; import static java.nio.charset.StandardCharsets.UTF_8; @@ -118,7 +118,7 @@ public static Slice copyOf(Slice slice, int offset, int length) public static Slice wrappedBuffer(ByteBuffer buffer) { if (buffer.isDirect()) { - long address = getAddress(buffer); + long address = bufferAddress(buffer); return new Slice(null, address + buffer.position(), buffer.remaining(), buffer.capacity(), buffer); } diff --git a/src/test/java/io/airlift/slice/TestUnsafeSliceFactory.java b/src/test/java/io/airlift/slice/TestUnsafeSliceFactory.java index eb2d66a4..eec205dc 100644 --- a/src/test/java/io/airlift/slice/TestUnsafeSliceFactory.java +++ b/src/test/java/io/airlift/slice/TestUnsafeSliceFactory.java @@ -19,7 +19,7 @@ import java.nio.ByteBuffer; import java.security.Permission; -import static io.airlift.slice.JvmUtils.getAddress; +import static io.airlift.slice.JvmUtils.bufferAddress; import static io.airlift.slice.JvmUtils.unsafe; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; @@ -55,7 +55,7 @@ public void testRawAddressWithReference() { ByteBuffer buffer = ByteBuffer.allocateDirect(100); assertTrue(buffer.isDirect()); - long address = getAddress(buffer); + long address = bufferAddress(buffer); UnsafeSliceFactory factory = UnsafeSliceFactory.getInstance(); From 49e0be29da28cb9fe50ceec29c9e39caf5a681e6 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Sun, 30 Dec 2018 23:41:51 -0800 Subject: [PATCH 9/9] Remove private constructor usage for direct buffers The buffer is created by duplicating the original buffer that is referenced by the Slice object. This avoids illegal access warnings on JDK 9, but does not allow creating buffers for Slices created via UnsafeSliceFactory that access arbitrary memory locations. --- src/main/java/io/airlift/slice/JvmUtils.java | 13 ---------- src/main/java/io/airlift/slice/Slice.java | 26 ++++++++----------- src/test/java/io/airlift/slice/TestSlice.java | 20 ++++++++++++++ 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/main/java/io/airlift/slice/JvmUtils.java b/src/main/java/io/airlift/slice/JvmUtils.java index db2ee346..67cd5f6d 100644 --- a/src/main/java/io/airlift/slice/JvmUtils.java +++ b/src/main/java/io/airlift/slice/JvmUtils.java @@ -15,13 +15,8 @@ import sun.misc.Unsafe; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; -import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.nio.Buffer; -import java.nio.ByteBuffer; import static io.airlift.slice.Preconditions.checkArgument; import static sun.misc.Unsafe.ARRAY_BOOLEAN_INDEX_SCALE; @@ -35,7 +30,6 @@ final class JvmUtils { static final Unsafe unsafe; - static final MethodHandle newByteBuffer; private static final long ADDRESS_OFFSET; @@ -58,13 +52,6 @@ final class JvmUtils assertArrayIndexScale("Float", ARRAY_FLOAT_INDEX_SCALE, 4); assertArrayIndexScale("Double", ARRAY_DOUBLE_INDEX_SCALE, 8); - // fetch a method handle for the hidden constructor for DirectByteBuffer - Class directByteBufferClass = ClassLoader.getSystemClassLoader().loadClass("java.nio.DirectByteBuffer"); - Constructor constructor = directByteBufferClass.getDeclaredConstructor(long.class, int.class, Object.class); - constructor.setAccessible(true); - newByteBuffer = MethodHandles.lookup().unreflectConstructor(constructor) - .asType(MethodType.methodType(ByteBuffer.class, long.class, int.class, Object.class)); - // fetch the address field for direct buffers ADDRESS_OFFSET = unsafe.objectFieldOffset(Buffer.class.getDeclaredField("address")); } diff --git a/src/main/java/io/airlift/slice/Slice.java b/src/main/java/io/airlift/slice/Slice.java index c47703ed..77c257d9 100644 --- a/src/main/java/io/airlift/slice/Slice.java +++ b/src/main/java/io/airlift/slice/Slice.java @@ -24,7 +24,7 @@ import java.nio.ByteBuffer; import java.nio.charset.Charset; -import static io.airlift.slice.JvmUtils.newByteBuffer; +import static io.airlift.slice.JvmUtils.bufferAddress; import static io.airlift.slice.JvmUtils.unsafe; import static io.airlift.slice.Preconditions.checkArgument; import static io.airlift.slice.Preconditions.checkPositionIndexes; @@ -43,6 +43,7 @@ import static io.airlift.slice.SizeOf.sizeOfShortArray; import static java.lang.Math.min; import static java.lang.Math.multiplyExact; +import static java.lang.Math.toIntExact; import static java.lang.String.format; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; @@ -1209,21 +1210,16 @@ public ByteBuffer toByteBuffer(int index, int length) return ByteBuffer.wrap((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length).slice(); } - try { - return (ByteBuffer) newByteBuffer.invokeExact(address + index, length, (Object) reference); - } - catch (Throwable throwable) { - if (throwable instanceof Error) { - throw (Error) throwable; - } - if (throwable instanceof RuntimeException) { - throw (RuntimeException) throwable; - } - if (throwable instanceof InterruptedException) { - Thread.currentThread().interrupt(); - } - throw new RuntimeException(throwable); + if ((reference instanceof ByteBuffer) && ((ByteBuffer) reference).isDirect()) { + ByteBuffer buffer = (ByteBuffer) reference; + int position = toIntExact(address - bufferAddress(buffer)) + index; + buffer = buffer.duplicate(); + buffer.position(position); + buffer.limit(position + length); + return buffer.slice(); } + + throw new UnsupportedOperationException("Conversion to ByteBuffer not supported for this Slice"); } /** diff --git a/src/test/java/io/airlift/slice/TestSlice.java b/src/test/java/io/airlift/slice/TestSlice.java index 6fb71665..201b2229 100644 --- a/src/test/java/io/airlift/slice/TestSlice.java +++ b/src/test/java/io/airlift/slice/TestSlice.java @@ -838,6 +838,26 @@ public void testToByteBuffer() assertToByteBuffer(slice, original); } + @Test + public void testToByteBufferDirect() + { + byte[] original = "hello world".getBytes(UTF_8); + + ByteBuffer buffer = ByteBuffer.allocateDirect(original.length + 5); + buffer.putShort((short) 0xABCD); + buffer.put(original); + buffer.flip().position(2); + + Slice slice = Slices.wrappedBuffer(buffer); + assertEquals(slice.getBytes(), original); + + assertToByteBuffer(slice, original); + + // conversion does not depend on wrapped buffer position or limit + buffer.position(8).limit(12); + assertToByteBuffer(slice, original); + } + private static void assertToByteBuffer(Slice slice, byte[] original) { for (int index = 0; index < original.length; index++) {