From 286587d1516376f24072f9c88b1d68e46b01e642 Mon Sep 17 00:00:00 2001 From: pjulien Date: Mon, 18 Apr 2016 20:02:27 -0400 Subject: [PATCH 1/3] Fix for #3853 Removes the following allocations: - ``CharsetDecoder`` is reused between calls - ``CharBuffer#wrap`` removed in favor of heap based char buffer that is reused - Temporary ``char[]``, an intermediate copy inside ``StringCoding`` - Another ``char[]``, this is needed because ``StringCoding`` uses a ``CharBuffer`` internally but returns a ``char[]``. Extra characters need to be trimmed so this means yet another allocation - Yet another ``char[]`` directly from ``__string`` for non-heap based buffers Removes the following copies - No copy is performed to trim the allocation since a ``CharBuffer`` is used directly - For non-heap based byte buffers, removes the copy that was previously done in the __string function This does need to get the TLS entry which implies at least some contention on the thread object table and a fence. --- java/com/google/flatbuffers/Table.java | 50 +++++++++++++++++++------- 1 file changed, 38 insertions(+), 12 deletions(-) mode change 100644 => 100755 java/com/google/flatbuffers/Table.java diff --git a/java/com/google/flatbuffers/Table.java b/java/com/google/flatbuffers/Table.java old mode 100644 new mode 100755 index 36a9bcf90..f6ae4706b --- a/java/com/google/flatbuffers/Table.java +++ b/java/com/google/flatbuffers/Table.java @@ -19,6 +19,11 @@ package com.google.flatbuffers; import static com.google.flatbuffers.Constants.*; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.nio.CharBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.Charset; +import java.nio.charset.CharsetDecoder; +import java.nio.charset.CoderResult; /// @cond FLATBUFFERS_INTERNAL @@ -26,6 +31,13 @@ import java.nio.ByteOrder; * All tables in the generated code derive from this class, and add their own accessors. */ public class Table { + private final static ThreadLocal UTF8_DECODER = new ThreadLocal() { + @Override + protected CharsetDecoder initialValue() { + return Charset.forName("UTF-8").newDecoder(); + } + }; + private final static ThreadLocal CHAR_BUFFER = new ThreadLocal(); /** Used to hold the position of the `bb` buffer. */ protected int bb_pos; /** The underlying ByteBuffer to hold the data of the Table. */ @@ -71,20 +83,34 @@ public class Table { * @return Returns a `String` from the data stored inside the FlatBuffer at `offset`. */ protected String __string(int offset) { + CharsetDecoder decoder = UTF8_DECODER.get(); + decoder.reset(); + offset += bb.getInt(offset); - if (bb.hasArray()) { - return new String(bb.array(), bb.arrayOffset() + offset + SIZEOF_INT, bb.getInt(offset), - FlatBufferBuilder.utf8charset); - } else { - // We can't access .array(), since the ByteBuffer is read-only, - // off-heap or a memory map - ByteBuffer bb = this.bb.duplicate().order(ByteOrder.LITTLE_ENDIAN); - // We're forced to make an extra copy: - byte[] copy = new byte[bb.getInt(offset)]; - bb.position(offset + SIZEOF_INT); - bb.get(copy); - return new String(copy, 0, copy.length, FlatBufferBuilder.utf8charset); + ByteBuffer src = bb.duplicate().order(ByteOrder.LITTLE_ENDIAN); + int length = src.getInt(offset); + src.position(offset + SIZEOF_INT); + src.limit(offset + SIZEOF_INT + length); + + int required = (int)((float)length * decoder.maxCharsPerByte()); + CharBuffer dst = CHAR_BUFFER.get(); + if (dst == null || dst.capacity() < required) { + dst = CharBuffer.allocate(Math.max(required, 128)); + CHAR_BUFFER.set(dst); } + + dst.clear(); + + try { + CoderResult cr = decoder.decode(src, dst, true); + if (!cr.isUnderflow()) { + cr.throwException(); + } + } catch (CharacterCodingException x) { + throw new Error(x); + } + + return dst.flip().toString(); } /** From b0146b3d9a0e5f48269ad5186dc9c75023685b85 Mon Sep 17 00:00:00 2001 From: pjulien Date: Mon, 18 Apr 2016 20:19:56 -0400 Subject: [PATCH 2/3] fix file permission --- java/com/google/flatbuffers/Table.java | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 java/com/google/flatbuffers/Table.java diff --git a/java/com/google/flatbuffers/Table.java b/java/com/google/flatbuffers/Table.java old mode 100755 new mode 100644 From 9fb87f813b77f4dbb96ae8fa9420f5f77b247489 Mon Sep 17 00:00:00 2001 From: pjulien Date: Mon, 18 Apr 2016 21:43:28 -0400 Subject: [PATCH 3/3] size the allocation to the required bytes --- java/com/google/flatbuffers/Table.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/com/google/flatbuffers/Table.java b/java/com/google/flatbuffers/Table.java index f6ae4706b..408765420 100644 --- a/java/com/google/flatbuffers/Table.java +++ b/java/com/google/flatbuffers/Table.java @@ -95,7 +95,7 @@ public class Table { int required = (int)((float)length * decoder.maxCharsPerByte()); CharBuffer dst = CHAR_BUFFER.get(); if (dst == null || dst.capacity() < required) { - dst = CharBuffer.allocate(Math.max(required, 128)); + dst = CharBuffer.allocate(required); CHAR_BUFFER.set(dst); }