mobile/bind: avoid intermediate []rune copy converting Java string to Go

Converting a Go string to a string suitable use a specialized function,
UTF16Encode, that can encode the string directly to a malloc'ed buffer. That
way, only two copies are made when strings are passed from Go to Java; once
for UTF-8 to UTF-16 encoding and once for the creation of the Java String.

This CL implements the same optimization in the other direction, with a
UTF-16 to UTF-8 decoder implemented in C. Unfortunately, while calling into a
Go decoder also saves the extra copy, the Cgo overhead makes the calls much
slower for short strings.

To alleviate the risk of introducing decoding bugs, I've added the tests from
the encoding/utf16 package to SeqTest.

As a sideeffect, both Java and ObjC now always copy strings, regardless of
the argument mode. The cpy argument can therefore be removed from the string
conversion functions. Furthermore, the modeRetained and modeReturned modes
can be collapsed into just one.

While we're here, delete a leftover function from seq/strings.go that
wasn't removed when the old seq buffers went away.

Benchmarks, as compared with benchstat over 5 runs:

name                          old time/op  new time/op  delta
JavaStringShort               11.4µs ±13%  11.6µs ± 4%     ~     (p=0.859 n=10+5)
JavaStringShortDirect         19.5µs ± 9%  20.3µs ± 2%   +3.68%   (p=0.019 n=9+5)
JavaStringLong                 103µs ± 8%    24µs ± 4%  -77.13%   (p=0.001 n=9+5)
JavaStringLongDirect           113µs ± 9%    32µs ± 7%  -71.63%   (p=0.001 n=9+5)
JavaStringShortUnicode        11.1µs ±16%  10.7µs ± 5%     ~      (p=0.190 n=9+5)
JavaStringShortUnicodeDirect  19.6µs ± 7%  20.2µs ± 1%   +2.78%   (p=0.029 n=9+5)
JavaStringLongUnicode         97.1µs ± 9%  28.0µs ± 5%  -71.17%   (p=0.001 n=9+5)
JavaStringLongUnicodeDirect    105µs ±10%    34µs ± 5%  -67.23%   (p=0.002 n=8+5)
JavaStringRetShort            14.2µs ± 2%  13.9µs ± 1%   -2.15%   (p=0.006 n=8+5)
JavaStringRetShortDirect      20.8µs ± 2%  20.4µs ± 2%     ~      (p=0.065 n=8+5)
JavaStringRetLong             42.2µs ± 9%  42.4µs ± 3%     ~      (p=0.190 n=9+5)
JavaStringRetLongDirect       51.2µs ±21%  50.8µs ± 8%     ~      (p=0.518 n=9+5)
GoStringShort                 23.4µs ± 7%  22.5µs ± 3%   -3.55%   (p=0.019 n=9+5)
GoStringLong                  51.9µs ± 9%  53.1µs ± 3%     ~      (p=0.240 n=9+5)
GoStringShortUnicode          24.2µs ± 6%  22.8µs ± 1%   -5.54%   (p=0.002 n=9+5)
GoStringLongUnicode           58.6µs ± 8%  57.6µs ± 3%     ~      (p=0.518 n=9+5)
GoStringRetShort              27.6µs ± 1%  23.2µs ± 2%  -15.87%   (p=0.003 n=7+5)
GoStringRetLong                129µs ±12%    33µs ± 2%  -74.03%  (p=0.001 n=10+5)

Change-Id: Icb9481981493ffca8defed9fb80a9433d6048937
Reviewed-on: https://go-review.googlesource.com/20250
Reviewed-by: David Crawshaw <crawshaw@golang.org>

bind/java/SeqBench.java

@@ -84,6 +84,16 @@ public class SeqBench extends InstrumentationTestCase {
           Benchmark.String(Benchmark.LongStringUnicode);
         }
       });
+      benchmarks.put("StringRetShort", new Runnable() {
+        @Override public void run() {
+          Benchmark.StringRetShort();
+        }
+      });
+      benchmarks.put("StringRetLong", new Runnable() {
+        @Override public void run() {
+          Benchmark.StringRetLong();
+        }
+      });
       final byte[] shortSlice = Benchmark.getShortSlice();
       benchmarks.put("SliceShort", new Runnable() {
         @Override public void run() {
@@ -138,6 +148,12 @@ public class SeqBench extends InstrumentationTestCase {
     }
     @Override public void Slice(byte[] s) {
     }
+	@Override public String StringRetShort() {
+		return Benchmark.ShortString;
+	}
+	@Override public String StringRetLong() {
+		return Benchmark.LongString;
+	}
   }
 
   public void testBenchmark() {

bind/java/SeqTest.java

@@ -119,9 +119,25 @@ public class SeqTest extends InstrumentationTestCase {
     String[] tests = new String[]{
       "abcxyz09{}",
       "Hello, 世界",
-      "\uffff\uD800\uDC00\uD800\uDC01\uD808\uDF45\uDBFF\uDFFF"};
-    for (String want : tests) {
-      String got = Testpkg.StrDup(want);
+      "\uffff\uD800\uDC00\uD800\uDC01\uD808\uDF45\uDBFF\uDFFF",
+      // From Go std lib tests in unicode/utf16/utf16_test.go
+      "\u0001\u0002\u0003\u0004",
+      "\uffff\ud800\udc00\ud800\udc01\ud808\udf45\udbff\udfff",
+      "\ud800a",
+      "\udfff"
+    };
+    String[] wants = new String[]{
+      "abcxyz09{}",
+      "Hello, 世界",
+      "\uffff\uD800\uDC00\uD800\uDC01\uD808\uDF45\uDBFF\uDFFF",
+      "\u0001\u0002\u0003\u0004",
+      "\uffff\ud800\udc00\ud800\udc01\ud808\udf45\udbff\udfff",
+      "\ufffda",
+      "\ufffd"
+    };
+    for (int i = 0; i < tests.length; i++) {
+      String got = Testpkg.StrDup(tests[i]);
+      String want = wants[i];
       assertEquals("Strings should match", want, got);
     }
   }

bind/java/seq.h

@@ -14,12 +14,15 @@
 
 // Platform specific types
 typedef struct nstring {
-    void *chars; // utf16 encoded
-    jsize len; // length in bytes
+	// UTF16 or UTF8 Encoded string. When converting from Java string to Go
+	// string, UTF16. When converting from Go to Java, UTF8.
+	void *chars;
+	// length in bytes, regardless of encoding
+	jsize len;
 } nstring;
 typedef struct nbyteslice {
-    void *ptr;
-    jsize len;
+	void *ptr;
+	jsize len;
 } nbyteslice;
 typedef jlong nint;
 
@@ -34,7 +37,7 @@ extern jbyteArray go_seq_to_java_bytearray(JNIEnv *env, nbyteslice s, int copy);
 extern nbyteslice go_seq_from_java_bytearray(JNIEnv *env, jbyteArray s, int copy);
 
 extern jstring go_seq_to_java_string(JNIEnv *env, nstring str);
-extern nstring go_seq_from_java_string(JNIEnv *env, jstring s, int copy);
+extern nstring go_seq_from_java_string(JNIEnv *env, jstring s);
 
 // push_local_frame retrieves or creates the JNIEnv* for the current thread
 // and pushes a JNI reference frame. Must be matched with call to pop_local_frame.

bind/java/seq_android.c.support

@@ -90,30 +90,108 @@ jbyteArray go_seq_to_java_bytearray(JNIEnv *env, nbyteslice s, int copy) {
 	return res;
 }
 
-nstring go_seq_from_java_string(JNIEnv *env, jstring str, int copy) {
+#define surr1 0xd800
+#define surr2 0xdc00
+#define surr3 0xe000
+
+// Unicode replacement character
+#define replacementChar 0xFFFD
+
+#define rune1Max ((1<<7) - 1)
+#define rune2Max ((1<<11) - 1)
+#define rune3Max ((1<<16) - 1)
+// Maximum valid Unicode code point.
+#define MaxRune 0x0010FFFF
+
+#define surrogateMin 0xD800
+#define surrogateMax 0xDFFF
+// 0011 1111
+#define maskx 0x3F
+// 1000 0000
+#define tx 0x80
+// 1100 0000
+#define t2 0xC0
+// 1110 0000
+#define t3 0xE0
+// 1111 0000
+#define t4 0xF0
+
+// encode_rune writes into p (which must be large enough) the UTF-8 encoding
+// of the rune. It returns the number of bytes written.
+static int encode_rune(uint8_t *p, uint32_t r) {
+	if (r <= rune1Max) {
+		p[0] = (uint8_t)r;
+		return 1;
+	} else if (r <= rune2Max) {
+		p[0] = t2 | (uint8_t)(r>>6);
+		p[1] = tx | (((uint8_t)(r))&maskx);
+		return 2;
+	} else {
+		if (r > MaxRune || (surrogateMin <= r && r <= surrogateMax)) {
+			r = replacementChar;
+		}
+		if (r <= rune3Max) {
+			p[0] = t3 | (uint8_t)(r>>12);
+			p[1] = tx | (((uint8_t)(r>>6))&maskx);
+			p[2] = tx | (((uint8_t)(r))&maskx);
+			return 3;
+		} else {
+			p[0] = t4 | (uint8_t)(r>>18);
+			p[1] = tx | (((uint8_t)(r>>12))&maskx);
+			p[2] = tx | (((uint8_t)(r>>6))&maskx);
+			p[3] = tx | (((uint8_t)(r))&maskx);
+			return 4;
+		}
+	}
+}
+
+// utf16_decode decodes an array of UTF16 characters to a UTF-8 encoded
+// nstring copy. The support functions and utf16_decode itself are heavily
+// based on the unicode/utf8 and unicode/utf16 Go packages.
+static nstring utf16_decode(jchar *chars, jsize len) {
+	jsize worstCaseLen = 4*len;
+	uint8_t *buf = malloc(worstCaseLen);
+	if (buf == NULL) {
+		LOG_FATAL("utf16Decode: malloc failed");
+	}
+	jsize nsrc = 0;
+	jsize ndst = 0;
+	while (nsrc < len) {
+		uint32_t r = chars[nsrc];
+		nsrc++;
+		if (surr1 <= r && r < surr2 && nsrc < len) {
+			uint32_t r2 = chars[nsrc];
+			if (surr2 <= r2 && r2 < surr3) {
+				nsrc++;
+				r = (((r-surr1)<<10) | (r2 - surr2)) + 0x10000;
+			}
+		}
+		if (ndst + 4 > worstCaseLen) {
+			LOG_FATAL("utf16Decode: buffer overflow");
+		}
+		ndst += encode_rune(buf + ndst, r);
+	}
+	struct nstring res = {chars: buf, len: ndst};
+	return res;
+}
+
+nstring go_seq_from_java_string(JNIEnv *env, jstring str) {
 	struct nstring res = {NULL, 0};
 	if (str == NULL) {
 		return res;
 	}
 	jsize nchars = (*env)->GetStringLength(env, str);
+	if (nchars == 0) {
+		return res;
+	}
 	jchar *chars = (jchar *)(*env)->GetStringChars(env, str, NULL);
 	if (chars == NULL) {
 		LOG_FATAL("GetStringChars failed");
 		return res;
 	}
-	if (copy) {
-		void *arr_copy = malloc(nchars*2);
-		if (arr_copy == NULL) {
-			LOG_FATAL("malloc failed");
-			return res;
-		}
-		memcpy(arr_copy, chars, nchars*2);
-		(*env)->ReleaseStringChars(env, str, chars);
-		chars = (jchar *)arr_copy;
-	}
-	res.chars = chars;
-	res.len = nchars;
-	return res;
+	nstring nstr = utf16_decode(chars, nchars);
+	(*env)->ReleaseStringChars(env, str, chars);
+	return nstr;
 }
 
 nbyteslice go_seq_from_java_bytearray(JNIEnv *env, jbyteArray arr, int copy) {
@@ -174,7 +252,7 @@ jobject go_seq_from_refnum(JNIEnv *env, int32_t refnum, jclass proxy_class, jmet
 
 // go_seq_to_java_string converts a nstring to a jstring.
 jstring go_seq_to_java_string(JNIEnv *env, nstring str) {
-	jstring s = (*env)->NewString(env, str.chars, str.len);
+	jstring s = (*env)->NewString(env, str.chars, str.len/2);
 	if (str.chars != NULL) {
 		free(str.chars);
 	}

bind/java/seq_android.go.support

@@ -15,7 +15,6 @@ package gomobile_bind
 //#include "seq.h"
 import "C"
 import (
-	"unicode/utf16"
 	"unsafe"
 
 	"golang.org/x/mobile/bind/seq"
@@ -27,13 +26,12 @@ func DestroyRef(refnum C.int32_t) {
 	seq.Delete(int32(refnum))
 }
 
-// encodeString encodes a Go string to utf16 and returns a Java string as a nstring
-// containing the jstring.
+// encodeString returns a copy of a Go string as a UTF16 encoded nstring.
+// The returned data is freed in go_seq_to_java_string.
+//
 // encodeString uses UTF16 as the intermediate format. Note that UTF8 is an obvious
 // alternative, but JNI only supports a C-safe variant of UTF8 (modified UTF8).
-// The returned data is always a copy, regardless of cpy, and will be freed in
-// go_seq_to_java_string.
-func encodeString(s string, cpy bool) C.nstring {
+func encodeString(s string) C.nstring {
 	n := C.int(len(s))
 	if n == 0 {
 		return C.nstring{}
@@ -46,20 +44,18 @@ func encodeString(s string, cpy bool) C.nstring {
 	}
 	chars := (*[1<<30 - 1]uint16)(unsafe.Pointer(utf16buf))[:worstCaseLen/2 : worstCaseLen/2]
 	nchars := seq.UTF16Encode(s, chars)
-	return C.nstring{chars: unsafe.Pointer(utf16buf), len: C.jsize(nchars)}
+	return C.nstring{chars: unsafe.Pointer(utf16buf), len: C.jsize(nchars*2)}
 }
 
-// decodeString decodes a nstring (jstring) to a Go string.
-// If cpy is set, the string contains a copy and is freed.
-func decodeString(str C.nstring, cpy bool) string {
+// decodeString decodes a UTF8 encoded nstring to a Go string. The data
+// in str is freed after use.
+func decodeString(str C.nstring) string {
 	if str.chars == nil {
 		return ""
 	}
-	chars := (*[1<<30 - 1]uint16)(unsafe.Pointer(str.chars))[:str.len]
-	s := string(utf16.Decode(chars)) // TODO: avoid the []rune allocation
-	if cpy {
-		C.free(str.chars)
-	}
+	chars := (*[1<<31 - 1]byte)(str.chars)[:str.len]
+	s := string(chars)
+	C.free(str.chars)
 	return s
 }