alex-plekhanov commented on code in PR #13529: URL: https://github.com/apache/ignite/pull/13529#discussion_r3988905537
########## modules/core/src/test/java/org/apache/ignite/internal/binary/StringWriterSelfTest.java: ########## @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.binary; + +import java.util.Arrays; +import java.util.concurrent.ThreadLocalRandom; +import org.apache.ignite.internal.binary.streams.BinaryOutputStream; +import org.apache.ignite.internal.binary.streams.BinaryStreams; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +import static java.nio.charset.StandardCharsets.UTF_8; + +/** + * Tests that {@link StringWriter} output is byte-identical to serialization of the {@link String#getBytes()} result, + * which was used before zero-copy string serialization was introduced. + */ +public class StringWriterSelfTest extends GridCommonAbstractTest { + /** */ + public static final int ASCII_MAX = 0x80; + + /** */ + public static final int LATIN1_MAX = 0x100; + + /** */ + public static final int TWO_BYTES_MAX = 0x800; + + /** */ + public static final int THREE_BYTES_MAX = 0xD800; + + /** */ + public static final int FOUR_BYTES_MAX = 0xE000; + + /** */ + public static final int FOUR_BYTES_HIGH_BOUND = 0x10000; Review Comment: Let's rename to CHAR_HIGH_BOUND, since chars near this bound are converted to 3 bytes UTF-8 ########## modules/core/src/test/java/org/apache/ignite/internal/binary/StringWriterSelfTest.java: ########## @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.binary; + +import java.util.Arrays; +import java.util.concurrent.ThreadLocalRandom; +import org.apache.ignite.internal.binary.streams.BinaryOutputStream; +import org.apache.ignite.internal.binary.streams.BinaryStreams; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +import static java.nio.charset.StandardCharsets.UTF_8; + +/** + * Tests that {@link StringWriter} output is byte-identical to serialization of the {@link String#getBytes()} result, + * which was used before zero-copy string serialization was introduced. + */ +public class StringWriterSelfTest extends GridCommonAbstractTest { + /** */ + public static final int ASCII_MAX = 0x80; + + /** */ + public static final int LATIN1_MAX = 0x100; + + /** */ + public static final int TWO_BYTES_MAX = 0x800; + + /** */ + public static final int THREE_BYTES_MAX = 0xD800; + + /** */ + public static final int FOUR_BYTES_MAX = 0xE000; + + /** */ + public static final int FOUR_BYTES_HIGH_BOUND = 0x10000; + + /** Tests for all encoder paths: ASCII bulk copy, Latin-1, generic UTF-16 and malformed surrogates. */ + @Test + public void testCorpus() { + String[] cases = { + "", + "a", + "?", + "abcdefghijklmnopqrstuvwxyz0123456789", // Long ASCII: exercises the 8-byte stride scan and bulk copy. + "caf\u00e9", // Latin-1 with a negative byte. + "\u00ff\u0080\u00a0", // Latin-1, negative bytes only. + "\u041f\u0440\u0438\u0432\u0435\u0442", // Cyrillic: 2-byte UTF-8 sequences. + "\u0800\u1234\uffff", // 3-byte UTF-8 sequences. + "\ud83d\ude00", // Emoji: valid surrogate pair. + "a\ud83d\ude00b\u00e9\u0416\u0001", // Mixed content. + "\ud800", // Lone high surrogate. + "\udc00", // Lone low surrogate. + "a\ud800", // High surrogate at the end. + "\ud800a", // High surrogate followed by a regular char. + "\ud800\ud800", // Two high surrogates. + "\udc00\ud800", // Low surrogate before a high one. + "\u0000", // NUL char. + "nul\u0000nul" + }; + + for (String str : cases) + check(str); + } + + /** Randomized differential test against {@link String#getBytes()}. */ + @Test + public void testRandomStrings() { + ThreadLocalRandom rnd = ThreadLocalRandom.current(); + + for (int iter = 0; iter < 100; iter++) { + StringBuilder sb = new StringBuilder(1 + rnd.nextInt(42)); + + for (int i = 0; i < sb.capacity(); i++) { + int bucket = rnd.nextInt(100); + + char c; + + if (bucket < 40) + // ASCII. + c = (char)rnd.nextInt(ASCII_MAX); + else if (bucket < 55) + // Latin-1. + c = (char)(ASCII_MAX + rnd.nextInt(LATIN1_MAX - ASCII_MAX)); + else if (bucket < 65) + // Other 2-byte chars. + c = (char)(LATIN1_MAX + rnd.nextInt(TWO_BYTES_MAX - LATIN1_MAX)); + else if (bucket < 75) + // 3-byte chars. + c = (char)(TWO_BYTES_MAX + rnd.nextInt(THREE_BYTES_MAX - TWO_BYTES_MAX)); + else if (bucket < 90) + // Surrogates, mostly malformed. + c = (char)(THREE_BYTES_MAX + rnd.nextInt(FOUR_BYTES_MAX - THREE_BYTES_MAX)); + else + // 3-byte chars above the surrogate range. + c = (char)(FOUR_BYTES_MAX + rnd.nextInt(FOUR_BYTES_HIGH_BOUND - FOUR_BYTES_MAX)); + + sb.append(c); + } + + assertFalse(sb.isEmpty()); + + check(sb.toString()); + } + } + + /** Tests strings whose UTF-8 form is larger than the stream's minimal capacity. */ + @Test + public void testLargeStrings() { + int len = 100_000; + + StringBuilder ascii = new StringBuilder(len); + StringBuilder latin1 = new StringBuilder(len); + StringBuilder cyrillic = new StringBuilder(len); + StringBuilder mixed = new StringBuilder(len); Review Comment: 6 chars is added on each iteration, len * 6 should be reserved ########## modules/binary/impl/src/main/java/org/apache/ignite/internal/binary/StringWriter.java: ########## @@ -0,0 +1,388 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.binary; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import org.apache.ignite.IgniteCommonsSystemProperties; +import org.apache.ignite.internal.binary.streams.BinaryOutputStream; +import org.apache.ignite.internal.util.GridUnsafe; +import org.apache.ignite.lang.IgniteBiTuple; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.ignite.internal.binary.BinaryWriterExImpl.ZERO_COPY; + +/** + * Writes {@link String} values to a {@link BinaryOutputStream} in UTF-8 without allocation of temporary byte arrays. + * + * @see IgniteCommonsSystemProperties#IGNITE_BINARY_STRING_ZERO_COPY + */ +public final class StringWriter { + /** Latin-1 value of the {@code java.lang.String#coder} field. */ + private static final byte LATIN1 = 0; + + /** Offset of the {@code java.lang.String#value} field, or {@code -1} if the compact string fast path is unavailable. */ + private static final long STR_VALUE_OFF; + + /** Offset of the {@code java.lang.String#coder} field, or {@code -1} if the compact string fast path is unavailable. */ + private static final long STR_CODER_OFF; + + static { + IgniteBiTuple<Long, Long> result = fieldsOffsets(); + + STR_VALUE_OFF = result.get1(); + STR_CODER_OFF = result.get2(); + } + + /** + * Handle of the intrinsified {@code java.lang.StringCoding#hasNegatives}, or {@code null} if unavailable. + * The intrinsic scans the array with SIMD instructions, far faster than any scalar loop. + */ + private static final MethodHandle HAS_NEGATIVES = hasNegatives(); + + /** */ + private StringWriter() { + // No-op. + } + + /** + * Writes a string to the output stream. + * + * @param val Value. + * @param out Output stream. + */ + public static void write(@NotNull String val, BinaryOutputStream out) { + // 1 byte for `GridBinaryMarshaller.STRING` and integer (4 bytes) for length. + out.unsafeEnsure(1 + 4); + out.unsafeWriteByte(GridBinaryMarshaller.STRING); + + int lenPos = out.position(); + + out.unsafePosition(out.position() + 4); + + int written; + + byte[] latin1 = latin1Value(val); + + if (latin1 != null) { + if (out.hasArray()) { + if (!hasNegatives(latin1)) { + out.unsafeEnsure(latin1.length); + // Pure ASCII: UTF-8 representation matches the internal array, copy it as-is. + System.arraycopy(latin1, 0, out.array(), out.position(), latin1.length); + + written = latin1.length; + } + else + written = encodeLatin1(latin1, out); + + out.unsafePosition(out.position() + written); + } + else + written = writeLatin1(latin1, out); + } + else { + // Allocating memory for worst case - 3 bytes per char. + out.unsafeEnsure(Math.multiplyExact(3, val.length())); + + if (out.hasArray()) { + written = encodeChars(val, out); + + out.unsafePosition(out.position() + written); + } + else + written = writeChars(val, out); + } + + out.unsafeWriteInt(lenPos, written); + } + + /** + * Writes a Latin-1 encoded string value to the stream. + * + * @param val Internal Latin-1 array of the string. + * @param out Output stream. + * @return Number of bytes written. + */ + private static int writeLatin1(byte[] val, BinaryOutputStream out) { + out.unsafeEnsure(Math.addExact(val.length, val.length)); + + int utfLen = 0; + + for (int i = 0; i < val.length; i++) { + byte b = val[i]; + + if (b >= 0) { + out.unsafeWriteByte(b); + + utfLen++; + } + else { + int c = b & 0b1111_1111; + + out.unsafeWriteByte((byte)(0b1100_0000 | (c >> 6))); + out.unsafeWriteByte((byte)(0b1000_0000 | (c & 0b0011_1111))); + + utfLen += 2; + } + } + + return utfLen; + } + + /** + * Encodes a Latin-1 string value to the buffer as UTF-8. + * + * @param val Internal Latin-1 array of the string. + * @param out Output stream. + * @return Count of written bytes. + */ + private static int encodeLatin1(byte[] val, BinaryOutputStream out) { + out.unsafeEnsure(Math.addExact(val.length, val.length)); + + byte[] buf = out.array(); + + long off = out.position() + GridUnsafe.BYTE_ARR_OFF; + + for (int i = 0; i < val.length; i++) { + byte b = val[i]; + + if (b >= 0) + GridUnsafe.putByte(buf, off++, b); Review Comment: Why `GridUnsafe.putByte(buf, off++, b)` instead of simple `but[off++] = b`? Performance issues? Why `getByte` instead of `val[I]` is not used then? Also, looks like `writeLatin1()` can be used instead of `encodeLatin()` without any major performance issues. and code can be deduplicated. ########## modules/binary/impl/src/main/java/org/apache/ignite/internal/binary/StringWriter.java: ########## @@ -0,0 +1,388 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.binary; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import org.apache.ignite.IgniteCommonsSystemProperties; +import org.apache.ignite.internal.binary.streams.BinaryOutputStream; +import org.apache.ignite.internal.util.GridUnsafe; +import org.apache.ignite.lang.IgniteBiTuple; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.ignite.internal.binary.BinaryWriterExImpl.ZERO_COPY; + +/** + * Writes {@link String} values to a {@link BinaryOutputStream} in UTF-8 without allocation of temporary byte arrays. + * + * @see IgniteCommonsSystemProperties#IGNITE_BINARY_STRING_ZERO_COPY + */ +public final class StringWriter { + /** Latin-1 value of the {@code java.lang.String#coder} field. */ + private static final byte LATIN1 = 0; + + /** Offset of the {@code java.lang.String#value} field, or {@code -1} if the compact string fast path is unavailable. */ + private static final long STR_VALUE_OFF; + + /** Offset of the {@code java.lang.String#coder} field, or {@code -1} if the compact string fast path is unavailable. */ + private static final long STR_CODER_OFF; + + static { + IgniteBiTuple<Long, Long> result = fieldsOffsets(); + + STR_VALUE_OFF = result.get1(); + STR_CODER_OFF = result.get2(); + } + + /** + * Handle of the intrinsified {@code java.lang.StringCoding#hasNegatives}, or {@code null} if unavailable. + * The intrinsic scans the array with SIMD instructions, far faster than any scalar loop. + */ + private static final MethodHandle HAS_NEGATIVES = hasNegatives(); + + /** */ + private StringWriter() { + // No-op. + } + + /** + * Writes a string to the output stream. + * + * @param val Value. + * @param out Output stream. + */ + public static void write(@NotNull String val, BinaryOutputStream out) { + // 1 byte for `GridBinaryMarshaller.STRING` and integer (4 bytes) for length. + out.unsafeEnsure(1 + 4); + out.unsafeWriteByte(GridBinaryMarshaller.STRING); + + int lenPos = out.position(); + + out.unsafePosition(out.position() + 4); + + int written; + + byte[] latin1 = latin1Value(val); + + if (latin1 != null) { + if (out.hasArray()) { + if (!hasNegatives(latin1)) { + out.unsafeEnsure(latin1.length); + // Pure ASCII: UTF-8 representation matches the internal array, copy it as-is. + System.arraycopy(latin1, 0, out.array(), out.position(), latin1.length); + + written = latin1.length; + } + else + written = encodeLatin1(latin1, out); + + out.unsafePosition(out.position() + written); + } + else + written = writeLatin1(latin1, out); + } + else { + // Allocating memory for worst case - 3 bytes per char. + out.unsafeEnsure(Math.multiplyExact(3, val.length())); + + if (out.hasArray()) { + written = encodeChars(val, out); + + out.unsafePosition(out.position() + written); + } + else + written = writeChars(val, out); + } + + out.unsafeWriteInt(lenPos, written); + } + + /** + * Writes a Latin-1 encoded string value to the stream. + * + * @param val Internal Latin-1 array of the string. + * @param out Output stream. + * @return Number of bytes written. + */ + private static int writeLatin1(byte[] val, BinaryOutputStream out) { + out.unsafeEnsure(Math.addExact(val.length, val.length)); + + int utfLen = 0; + + for (int i = 0; i < val.length; i++) { + byte b = val[i]; + + if (b >= 0) { + out.unsafeWriteByte(b); + + utfLen++; + } + else { + int c = b & 0b1111_1111; + + out.unsafeWriteByte((byte)(0b1100_0000 | (c >> 6))); + out.unsafeWriteByte((byte)(0b1000_0000 | (c & 0b0011_1111))); + + utfLen += 2; + } + } + + return utfLen; + } + + /** + * Encodes a Latin-1 string value to the buffer as UTF-8. + * + * @param val Internal Latin-1 array of the string. + * @param out Output stream. + * @return Count of written bytes. + */ + private static int encodeLatin1(byte[] val, BinaryOutputStream out) { + out.unsafeEnsure(Math.addExact(val.length, val.length)); + + byte[] buf = out.array(); + + long off = out.position() + GridUnsafe.BYTE_ARR_OFF; + + for (int i = 0; i < val.length; i++) { + byte b = val[i]; + + if (b >= 0) + GridUnsafe.putByte(buf, off++, b); + else { + int c = b & 0xFF; + + GridUnsafe.putByte(buf, off++, (byte)(0b1100_0000 | (c >> 6))); + GridUnsafe.putByte(buf, off++, (byte)(0b1000_0000 | (c & 0b0011_1111))); + } + } + + return (int)(off - GridUnsafe.BYTE_ARR_OFF - out.position()); + } + + /** + * Writes string chars UTF-8 encoded to the stream. Replicates {@code String#getBytes(UTF_8)} behavior exactly, + * including replacement of malformed surrogates with {@code '?'}. Stream capacity must be ensured by the caller. + * + * @param val Value. + * @param out Output stream. + * @return Number of bytes written. + */ + private static int writeChars(String val, BinaryOutputStream out) { + int len = val.length(); + int utfLen = 0; + + for (int i = 0; i < len; i++) { + char c = val.charAt(i); + + if (c < 0x80) { + out.unsafeWriteByte((byte)c); + + utfLen++; Review Comment: Can be calculated once, using out.position() ########## modules/core/src/test/java/org/apache/ignite/internal/binary/StringWriterSelfTest.java: ########## @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.binary; + +import java.util.Arrays; +import java.util.concurrent.ThreadLocalRandom; +import org.apache.ignite.internal.binary.streams.BinaryOutputStream; +import org.apache.ignite.internal.binary.streams.BinaryStreams; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +import static java.nio.charset.StandardCharsets.UTF_8; + +/** + * Tests that {@link StringWriter} output is byte-identical to serialization of the {@link String#getBytes()} result, + * which was used before zero-copy string serialization was introduced. + */ +public class StringWriterSelfTest extends GridCommonAbstractTest { + /** */ + public static final int ASCII_MAX = 0x80; + + /** */ + public static final int LATIN1_MAX = 0x100; + + /** */ + public static final int TWO_BYTES_MAX = 0x800; + + /** */ + public static final int THREE_BYTES_MAX = 0xD800; Review Comment: Character.MIN_SURROGATE? ########## modules/binary/impl/src/main/java/org/apache/ignite/internal/binary/StringWriter.java: ########## @@ -0,0 +1,388 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.binary; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import org.apache.ignite.IgniteCommonsSystemProperties; +import org.apache.ignite.internal.binary.streams.BinaryOutputStream; +import org.apache.ignite.internal.util.GridUnsafe; +import org.apache.ignite.lang.IgniteBiTuple; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.ignite.internal.binary.BinaryWriterExImpl.ZERO_COPY; + +/** + * Writes {@link String} values to a {@link BinaryOutputStream} in UTF-8 without allocation of temporary byte arrays. + * + * @see IgniteCommonsSystemProperties#IGNITE_BINARY_STRING_ZERO_COPY + */ +public final class StringWriter { + /** Latin-1 value of the {@code java.lang.String#coder} field. */ + private static final byte LATIN1 = 0; + + /** Offset of the {@code java.lang.String#value} field, or {@code -1} if the compact string fast path is unavailable. */ + private static final long STR_VALUE_OFF; + + /** Offset of the {@code java.lang.String#coder} field, or {@code -1} if the compact string fast path is unavailable. */ + private static final long STR_CODER_OFF; + + static { + IgniteBiTuple<Long, Long> result = fieldsOffsets(); + + STR_VALUE_OFF = result.get1(); + STR_CODER_OFF = result.get2(); + } + + /** + * Handle of the intrinsified {@code java.lang.StringCoding#hasNegatives}, or {@code null} if unavailable. + * The intrinsic scans the array with SIMD instructions, far faster than any scalar loop. + */ + private static final MethodHandle HAS_NEGATIVES = hasNegatives(); + + /** */ + private StringWriter() { + // No-op. + } + + /** + * Writes a string to the output stream. + * + * @param val Value. + * @param out Output stream. + */ + public static void write(@NotNull String val, BinaryOutputStream out) { + // 1 byte for `GridBinaryMarshaller.STRING` and integer (4 bytes) for length. + out.unsafeEnsure(1 + 4); + out.unsafeWriteByte(GridBinaryMarshaller.STRING); + + int lenPos = out.position(); + + out.unsafePosition(out.position() + 4); + + int written; + + byte[] latin1 = latin1Value(val); + + if (latin1 != null) { + if (out.hasArray()) { + if (!hasNegatives(latin1)) { + out.unsafeEnsure(latin1.length); + // Pure ASCII: UTF-8 representation matches the internal array, copy it as-is. + System.arraycopy(latin1, 0, out.array(), out.position(), latin1.length); + + written = latin1.length; + } + else + written = encodeLatin1(latin1, out); + + out.unsafePosition(out.position() + written); + } + else + written = writeLatin1(latin1, out); + } + else { + // Allocating memory for worst case - 3 bytes per char. + out.unsafeEnsure(Math.multiplyExact(3, val.length())); + + if (out.hasArray()) { + written = encodeChars(val, out); + + out.unsafePosition(out.position() + written); + } + else + written = writeChars(val, out); + } + + out.unsafeWriteInt(lenPos, written); + } + + /** + * Writes a Latin-1 encoded string value to the stream. + * + * @param val Internal Latin-1 array of the string. + * @param out Output stream. + * @return Number of bytes written. + */ + private static int writeLatin1(byte[] val, BinaryOutputStream out) { + out.unsafeEnsure(Math.addExact(val.length, val.length)); + + int utfLen = 0; + + for (int i = 0; i < val.length; i++) { + byte b = val[i]; + + if (b >= 0) { + out.unsafeWriteByte(b); + + utfLen++; + } + else { + int c = b & 0b1111_1111; + + out.unsafeWriteByte((byte)(0b1100_0000 | (c >> 6))); + out.unsafeWriteByte((byte)(0b1000_0000 | (c & 0b0011_1111))); + + utfLen += 2; + } + } + + return utfLen; + } + + /** + * Encodes a Latin-1 string value to the buffer as UTF-8. + * + * @param val Internal Latin-1 array of the string. + * @param out Output stream. + * @return Count of written bytes. + */ + private static int encodeLatin1(byte[] val, BinaryOutputStream out) { + out.unsafeEnsure(Math.addExact(val.length, val.length)); + + byte[] buf = out.array(); + + long off = out.position() + GridUnsafe.BYTE_ARR_OFF; + + for (int i = 0; i < val.length; i++) { + byte b = val[i]; + + if (b >= 0) + GridUnsafe.putByte(buf, off++, b); + else { + int c = b & 0xFF; + + GridUnsafe.putByte(buf, off++, (byte)(0b1100_0000 | (c >> 6))); + GridUnsafe.putByte(buf, off++, (byte)(0b1000_0000 | (c & 0b0011_1111))); + } + } + + return (int)(off - GridUnsafe.BYTE_ARR_OFF - out.position()); + } + + /** + * Writes string chars UTF-8 encoded to the stream. Replicates {@code String#getBytes(UTF_8)} behavior exactly, + * including replacement of malformed surrogates with {@code '?'}. Stream capacity must be ensured by the caller. + * + * @param val Value. + * @param out Output stream. + * @return Number of bytes written. + */ + private static int writeChars(String val, BinaryOutputStream out) { + int len = val.length(); + int utfLen = 0; + + for (int i = 0; i < len; i++) { + char c = val.charAt(i); + + if (c < 0x80) { + out.unsafeWriteByte((byte)c); + + utfLen++; + } + else if (c < 0x800) { + out.unsafeWriteByte((byte)(0b11_000000 | (c >> 6))); + out.unsafeWriteByte((byte)(0b10_000000 | (c & 0b00_111111))); + + utfLen += 2; + } + else if (!Character.isSurrogate(c)) { + out.unsafeWriteByte((byte)(0b1110_0000 | (c >> 12))); + out.unsafeWriteByte((byte)(0b1000_0000 | ((c >> 6) & 0b0011_1111))); + out.unsafeWriteByte((byte)(0b1000_0000 | (c & 0b0011_1111))); + + utfLen += 3; + } + else { + char c2; + + if (Character.isHighSurrogate(c) && i + 1 < len && Character.isLowSurrogate(c2 = val.charAt(i + 1))) { + int cp = Character.toCodePoint(c, c2); + + out.unsafeWriteByte((byte)(0b1111_0000 | (cp >> 18))); + out.unsafeWriteByte((byte)(0b1000_0000 | ((cp >> 12) & 0b0011_1111))); + out.unsafeWriteByte((byte)(0b1000_0000 | ((cp >> 6) & 0b0011_1111))); + out.unsafeWriteByte((byte)(0b1000_0000 | (cp & 0b0011_1111))); + + utfLen += 4; + i++; + } + else { + out.unsafeWriteByte((byte)'?'); + + utfLen++; + } + } + } + + return utfLen; + } + + /** + * Encodes string chars to the buffer as UTF-8. Replicates {@code String#getBytes(UTF_8)} behavior exactly, + * including replacement of malformed surrogates with {@code '?'}. Buffer capacity must be ensured by the caller. + * + * @param val Value. + * @param out Output stream. + * @return Count of written bytes. + */ + private static int encodeChars(String val, BinaryOutputStream out) { Review Comment: The same question here. Why do we need dedicated duplicated encodeChars()? Looks like writeChars can be used without any major performance issues. One extra arithmetic "add" per byte - your will never notice performance difference in any real world scenario. ########## modules/core/src/test/java/org/apache/ignite/internal/binary/StringWriterSelfTest.java: ########## @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.binary; + +import java.util.Arrays; +import java.util.concurrent.ThreadLocalRandom; +import org.apache.ignite.internal.binary.streams.BinaryOutputStream; +import org.apache.ignite.internal.binary.streams.BinaryStreams; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +import static java.nio.charset.StandardCharsets.UTF_8; + +/** + * Tests that {@link StringWriter} output is byte-identical to serialization of the {@link String#getBytes()} result, + * which was used before zero-copy string serialization was introduced. + */ +public class StringWriterSelfTest extends GridCommonAbstractTest { + /** */ + public static final int ASCII_MAX = 0x80; + + /** */ + public static final int LATIN1_MAX = 0x100; + + /** */ + public static final int TWO_BYTES_MAX = 0x800; + + /** */ + public static final int THREE_BYTES_MAX = 0xD800; + + /** */ + public static final int FOUR_BYTES_MAX = 0xE000; Review Comment: Character.MAX_SURROGATE - 1? ########## modules/commons/src/main/java/org/apache/ignite/IgniteCommonsSystemProperties.java: ########## @@ -38,6 +38,9 @@ public class IgniteCommonsSystemProperties { /** Default value of {@link IgniteCommonsSystemProperties#IGNITE_USE_BINARY_ARRAYS}. */ public static final boolean DFLT_IGNITE_USE_BINARY_ARRAYS = false; + /** Default value of {@link IgniteCommonsSystemProperties#IGNITE_BINARY_STRING_ZERO_COPY}. */ + public static final boolean DFLT_ZERO_COPY = true; Review Comment: Why do we need ability to disable it explicitly? Why can't we disable it only when related methods are not available via reflection? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
