From ba0a725146b4891fbac244e524f25a028d391ff2 Mon Sep 17 00:00:00 2001 From: Elias Naur Date: Fri, 4 Mar 2016 17:59:04 +0100 Subject: [PATCH] mobile/bind: avoid intermediate []rune copy converting Java string to Go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- bind/benchmark/benchmark.go | 14 ++++ bind/gen.go | 19 +---- bind/gengo.go | 18 ++--- bind/genjava.go | 26 +++--- bind/genobjc.go | 18 ++--- bind/java/SeqBench.java | 16 ++++ bind/java/SeqTest.java | 22 ++++- bind/java/seq.h | 13 +-- bind/java/seq_android.c.support | 108 +++++++++++++++++++++---- bind/java/seq_android.go.support | 26 +++--- bind/objc/seq_darwin.go.support | 11 +-- bind/seq/string.go | 9 +-- bind/testdata/basictypes.go.golden | 4 +- bind/testdata/interfaces.go.golden | 6 +- bind/testdata/interfaces.java.c.golden | 2 +- bind/testdata/issue10788.go.golden | 8 +- bind/testdata/issue10788.java.c.golden | 10 +-- bind/testdata/issue12328.go.golden | 4 +- bind/testdata/issue12328.java.c.golden | 5 +- bind/testdata/issue12403.go.golden | 16 ++-- bind/testdata/issue12403.java.c.golden | 11 +-- bind/testdata/structs.go.golden | 4 +- bind/testdata/try.go.golden | 2 +- bind/testdata/vars.go.golden | 4 +- bind/testdata/vars.java.c.golden | 5 +- 25 files changed, 230 insertions(+), 151 deletions(-) diff --git a/bind/benchmark/benchmark.go b/bind/benchmark/benchmark.go index 1d81ab7..2e56861 100644 --- a/bind/benchmark/benchmark.go +++ b/bind/benchmark/benchmark.go @@ -29,6 +29,8 @@ type Benchmarks interface { Ref(_ I) Manyargs(_, _, _, _, _, _, _, _, _, _ int) String(_ string) + StringRetShort() string + StringRetLong() string Slice(_ []byte) } @@ -94,6 +96,8 @@ func RunBenchmarks(b Benchmarks) { "StringLong", "StringShortUnicode", "StringLongUnicode", + "StringRetShort", + "StringRetLong", "SliceShort", "SliceLong", } @@ -118,6 +122,8 @@ func RunBenchmarks(b Benchmarks) { runGoBenchmark("StringLong", func() { b.String(LongString) }) runGoBenchmark("StringShortUnicode", func() { b.String(ShortStringUnicode) }) runGoBenchmark("StringLongUnicode", func() { b.String(LongStringUnicode) }) + runGoBenchmark("StringRetShort", func() { b.StringRetShort() }) + runGoBenchmark("StringRetLong", func() { b.StringRetLong() }) runGoBenchmark("SliceShort", func() { b.Slice(ShortSlice) }) runGoBenchmark("SliceLong", func() { b.Slice(LongSlice) }) } @@ -138,6 +144,14 @@ func Oneret() int { func String(_ string) { } +func StringRetShort() string { + return ShortString +} + +func StringRetLong() string { + return LongString +} + func Slice(_ []byte) { } diff --git a/bind/gen.go b/bind/gen.go index aac6842..6bd2e01 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -30,26 +30,15 @@ type ( const ( // modeTransient are for function arguments that // are not used after the function returns. - // Transient strings and byte slices don't need copying + // Transient byte slices don't need copying // when passed accross the language barrier. modeTransient varMode = iota - // modeRetained are for function arguments that are - // used after the function returns. Retained strings - // don't need an intermediate copy, while byte slices do. + // modeRetained are for returned values and for function + // arguments that are used after the function returns. + // Retained byte slices need an intermediate copy. modeRetained - // modeReturned are for values that are returned to the - // caller of a function. Returned values are always copied. - modeReturned ) -func (m varMode) copyString() bool { - return m == modeReturned -} - -func (m varMode) copySlice() bool { - return m == modeReturned || m == modeRetained -} - func (list ErrorList) Error() string { buf := new(bytes.Buffer) for i, err := range list { diff --git a/bind/gengo.go b/bind/gengo.go index 5a85e92..805b559 100644 --- a/bind/gengo.go +++ b/bind/gengo.go @@ -76,7 +76,7 @@ func (g *goGen) genFuncBody(o *types.Func, selectorLHS string) { for i := 0; i < res.Len(); i++ { pn := fmt.Sprintf("res_%d", i) - g.genWrite("_"+pn, pn, res.At(i).Type(), modeReturned) + g.genWrite("_"+pn, pn, res.At(i).Type(), modeRetained) } if res.Len() > 0 { g.Printf("return ") @@ -105,7 +105,7 @@ func (g *goGen) genWrite(toVar, fromVar string, t types.Type, mode varMode) { case *types.Basic: switch t.Kind() { case types.String: - g.Printf("%s := encodeString(%s, %v)\n", toVar, fromVar, mode.copyString()) + g.Printf("%s := encodeString(%s)\n", toVar, fromVar) case types.Bool: g.Printf("var %s C.%s = 0\n", toVar, g.cgoType(t)) g.Printf("if %s { %s = 1 }\n", fromVar, toVar) @@ -117,7 +117,7 @@ func (g *goGen) genWrite(toVar, fromVar string, t types.Type, mode varMode) { case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - g.Printf("%s := fromSlice(%s, %v)\n", toVar, fromVar, mode.copySlice()) + g.Printf("%s := fromSlice(%s, %v)\n", toVar, fromVar, mode == modeRetained) default: g.errorf("unsupported type: %s", t) } @@ -219,7 +219,7 @@ func (g *goGen) genStruct(obj *types.TypeName, T *types.Struct) { g.Indent() g.Printf("ref := _seq.FromRefNum(int32(refnum))\n") g.Printf("v := ref.Get().(*%s.%s).%s\n", g.pkg.Name(), obj.Name(), f.Name()) - g.genWrite("_v", "v", f.Type(), modeReturned) + g.genWrite("_v", "v", f.Type(), modeRetained) g.Printf("return _v\n") g.Outdent() g.Printf("}\n\n") @@ -257,7 +257,7 @@ func (g *goGen) genVar(o *types.Var) { g.Printf("func var_get%s_%s() C.%s {\n", g.pkgPrefix, o.Name(), g.cgoType(o.Type())) g.Indent() g.Printf("v := %s\n", v) - g.genWrite("_v", "v", o.Type(), modeReturned) + g.genWrite("_v", "v", o.Type(), modeRetained) g.Printf("return _v\n") g.Outdent() g.Printf("}\n") @@ -333,13 +333,13 @@ func (g *goGen) genInterface(obj *types.TypeName) { if res.Len() > 0 { if res.Len() == 1 { T := res.At(0).Type() - g.genRead("_res", "res", T, modeReturned) + g.genRead("_res", "res", T, modeRetained) retName = "_res" } else { var rvs []string for i := 0; i < res.Len(); i++ { rv := fmt.Sprintf("res_%d", i) - g.genRead(rv, fmt.Sprintf("res.r%d", i), res.At(i).Type(), modeReturned) + g.genRead(rv, fmt.Sprintf("res.r%d", i), res.At(i).Type(), modeRetained) rvs = append(rvs, rv) } retName = strings.Join(rvs, ", ") @@ -361,7 +361,7 @@ func (g *goGen) genRead(toVar, fromVar string, typ types.Type, mode varMode) { case *types.Basic: switch t.Kind() { case types.String: - g.Printf("%s := decodeString(%s, %v)\n", toVar, fromVar, mode.copyString()) + g.Printf("%s := decodeString(%s)\n", toVar, fromVar) case types.Bool: g.Printf("%s := %s != 0\n", toVar, fromVar) default: @@ -372,7 +372,7 @@ func (g *goGen) genRead(toVar, fromVar string, typ types.Type, mode varMode) { case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - g.Printf("%s := toSlice(%s, %v)\n", toVar, fromVar, mode.copySlice()) + g.Printf("%s := toSlice(%s, %v)\n", toVar, fromVar, mode == modeRetained) default: g.errorf("unsupported type: %s", t) } diff --git a/bind/genjava.go b/bind/genjava.go index ffdba86..3749bc2 100644 --- a/bind/genjava.go +++ b/bind/genjava.go @@ -404,7 +404,7 @@ func (g *javaGen) genJavaToC(varName string, t types.Type, mode varMode) { case *types.Basic: switch t.Kind() { case types.String: - g.Printf("nstring _%s = go_seq_from_java_string(env, %s, %d);\n", varName, varName, g.toCFlag(mode.copyString())) + g.Printf("nstring _%s = go_seq_from_java_string(env, %s);\n", varName, varName) default: g.Printf("%s _%s = (%s)%s;\n", g.cgoType(t), varName, g.cgoType(t), varName) } @@ -413,7 +413,7 @@ func (g *javaGen) genJavaToC(varName string, t types.Type, mode varMode) { case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - g.Printf("nbyteslice _%s = go_seq_from_java_bytearray(env, %s, %d);\n", varName, varName, g.toCFlag(mode.copySlice())) + g.Printf("nbyteslice _%s = go_seq_from_java_bytearray(env, %s, %d);\n", varName, varName, g.toCFlag(mode == modeRetained)) default: g.errorf("unsupported type: %s", t) } @@ -454,7 +454,7 @@ func (g *javaGen) genCToJava(toName, fromName string, t types.Type, mode varMode case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - g.Printf("jbyteArray %s = go_seq_to_java_bytearray(env, %s, %d);\n", toName, fromName, g.toCFlag(mode.copySlice())) + g.Printf("jbyteArray %s = go_seq_to_java_bytearray(env, %s, %d);\n", toName, fromName, g.toCFlag(mode == modeRetained)) default: g.errorf("unsupported type: %s", t) } @@ -579,7 +579,7 @@ func (g *javaGen) genJNIField(o *types.TypeName, f *types.Var) { g.Printf("int32_t o = go_seq_to_refnum(env, this);\n") g.Printf("%s r0 = ", g.cgoType(f.Type())) g.Printf("proxy%s_%s_%s_Get(o);\n", g.pkgPrefix, o.Name(), f.Name()) - g.genCToJava("_r0", "r0", f.Type(), modeReturned) + g.genCToJava("_r0", "r0", f.Type(), modeRetained) g.Printf("return _r0;\n") g.Outdent() g.Printf("}\n\n") @@ -602,7 +602,7 @@ func (g *javaGen) genJNIVar(o *types.Var) { g.Indent() g.Printf("%s r0 = ", g.cgoType(o.Type())) g.Printf("var_get%s_%s();\n", g.pkgPrefix, o.Name()) - g.genCToJava("_r0", "r0", o.Type(), modeReturned) + g.genCToJava("_r0", "r0", o.Type(), modeRetained) g.Printf("return _r0;\n") g.Outdent() g.Printf("}\n\n") @@ -650,7 +650,7 @@ func (g *javaGen) genJNIFunc(o *types.Func, sName string, proxy bool) { for i := 0; i < res.Len(); i++ { tn := fmt.Sprintf("_r%d", i) t := res.At(i).Type() - g.genCToJava(tn, fmt.Sprintf("%sr%d", resPrefix, i), t, modeReturned) + g.genCToJava(tn, fmt.Sprintf("%sr%d", resPrefix, i), t, modeRetained) } // Go backwards so that any exception is thrown before // the return. @@ -674,20 +674,12 @@ func (g *javaGen) genRelease(varName string, t types.Type, mode varMode) { } switch t := t.(type) { case *types.Basic: - switch t.Kind() { - case types.String: - if !mode.copyString() { - g.Printf("if (_%s.chars != NULL) {\n", varName) - g.Printf(" (*env)->ReleaseStringChars(env, %s, _%s.chars);\n", varName, varName) - g.Printf("}\n") - } - } case *types.Slice: switch e := t.Elem().(type) { case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - if !mode.copySlice() { + if mode == modeTransient { g.Printf("if (_%s.ptr != NULL) {\n", varName) g.Printf(" (*env)->ReleaseByteArrayElements(env, %s, _%s.ptr, 0);\n", varName, varName) g.Printf("}\n") @@ -727,14 +719,14 @@ func (g *javaGen) genMethodInterfaceProxy(oName string, m *types.Func) { var rets []string t := res.At(0).Type() if !isErrorType(t) { - g.genJavaToC("res", t, modeReturned) + g.genJavaToC("res", t, modeRetained) retName = "_res" rets = append(rets, retName) } if res.Len() == 2 || isErrorType(t) { g.Printf("jstring exc = go_seq_get_exception_message(env);\n") st := types.Typ[types.String] - g.genJavaToC("exc", st, modeReturned) + g.genJavaToC("exc", st, modeRetained) retName = "_exc" rets = append(rets, "_exc") } diff --git a/bind/genobjc.go b/bind/genobjc.go index 509c626..c047e0c 100644 --- a/bind/genobjc.go +++ b/bind/genobjc.go @@ -257,7 +257,7 @@ func (g *objcGen) genVarM(o *types.Var) { g.Indent() g.Printf("%s r0 = ", g.cgoType(o.Type())) g.Printf("var_get%s_%s();\n", g.pkgPrefix, o.Name()) - g.genRead("_r0", "r0", o.Type(), modeReturned) + g.genRead("_r0", "r0", o.Type(), modeRetained) g.Printf("return _r0;\n") g.Outdent() g.Printf("}\n\n") @@ -470,7 +470,7 @@ func (g *objcGen) genGetter(oName string, f *types.Var) { g.Printf("int32_t refnum = go_seq_go_to_refnum(self._ref);\n") g.Printf("%s r0 = ", g.cgoType(f.Type())) g.Printf("proxy%s_%s_%s_Get(refnum);\n", g.pkgPrefix, oName, f.Name()) - g.genRead("_r0", "r0", f.Type(), modeReturned) + g.genRead("_r0", "r0", f.Type(), modeRetained) g.Printf("return _r0;\n") g.Outdent() g.Printf("}\n\n") @@ -510,7 +510,7 @@ func (g *objcGen) genWrite(varName string, t types.Type, mode varMode) { case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - g.Printf("nbyteslice _%s = go_seq_from_objc_bytearray(%s, %d);\n", varName, varName, g.toCFlag(mode.copySlice())) + g.Printf("nbyteslice _%s = go_seq_from_objc_bytearray(%s, %d);\n", varName, varName, g.toCFlag(mode == modeRetained)) default: g.errorf("unsupported type: %s", t) } @@ -577,7 +577,7 @@ func (g *objcGen) genRead(toName, fromName string, t types.Type, mode varMode) { case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - g.Printf("NSData *%s = go_seq_to_objc_bytearray(%s, %d);\n", toName, fromName, g.toCFlag(mode.copySlice())) + g.Printf("NSData *%s = go_seq_to_objc_bytearray(%s, %d);\n", toName, fromName, g.toCFlag(mode == modeRetained)) default: g.errorf("unsupported type: %s", t) } @@ -635,7 +635,7 @@ func (g *objcGen) genFunc(s *funcSummary, objName string) { } for i, r := range s.retParams { - g.genRead("_"+r.name, fmt.Sprintf("%sr%d", resPrefix, i), r.typ, modeReturned) + g.genRead("_"+r.name, fmt.Sprintf("%sr%d", resPrefix, i), r.typ, modeRetained) } if !s.returnsVal() { @@ -754,7 +754,7 @@ func (g *objcGen) genInterfaceMethodProxy(obj *types.TypeName, m *types.Func) { if len(s.retParams) > 0 { if s.returnsVal() { // len(s.retParams) == 1 && s.retParams[0] != error p := s.retParams[0] - g.genWrite("returnVal", p.typ, modeReturned) + g.genWrite("returnVal", p.typ, modeRetained) g.Printf("return _returnVal;\n") } else { var rets []string @@ -775,10 +775,10 @@ func (g *objcGen) genInterfaceMethodProxy(obj *types.TypeName, m *types.Func) { g.Printf("}\n") g.Outdent() g.Printf("}\n") - g.genWrite(p.name+"_str", p.typ, modeReturned) + g.genWrite(p.name+"_str", p.typ, modeRetained) rets = append(rets, fmt.Sprintf("_%s_str", p.name)) } else { - g.genWrite(p.name, p.typ, modeReturned) + g.genWrite(p.name, p.typ, modeRetained) rets = append(rets, "_"+p.name) } } @@ -808,7 +808,7 @@ func (g *objcGen) genRelease(varName string, t types.Type, mode varMode) { case *types.Basic: switch e.Kind() { case types.Uint8: // Byte. - if !mode.copySlice() { + if mode == modeTransient { // If the argument was not mutable, go_seq_from_objc_bytearray created a copy. // Free it here. g.Printf("if (![%s isKindOfClass:[NSMutableData class]]) {\n", varName) diff --git a/bind/java/SeqBench.java b/bind/java/SeqBench.java index 46a0f74..90e75a3 100644 --- a/bind/java/SeqBench.java +++ b/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() { diff --git a/bind/java/SeqTest.java b/bind/java/SeqTest.java index c252957..474591a 100644 --- a/bind/java/SeqTest.java +++ b/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); } } diff --git a/bind/java/seq.h b/bind/java/seq.h index 3514175..c6e16d3 100644 --- a/bind/java/seq.h +++ b/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. diff --git a/bind/java/seq_android.c.support b/bind/java/seq_android.c.support index 42d63dd..b1a095a 100644 --- a/bind/java/seq_android.c.support +++ b/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); } diff --git a/bind/java/seq_android.go.support b/bind/java/seq_android.go.support index 2b6c50b..d112159 100644 --- a/bind/java/seq_android.go.support +++ b/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 } diff --git a/bind/objc/seq_darwin.go.support b/bind/objc/seq_darwin.go.support index a494084..3e4412e 100644 --- a/bind/objc/seq_darwin.go.support +++ b/bind/objc/seq_darwin.go.support @@ -31,9 +31,7 @@ func DestroyRef(refnum C.int32_t) { } // encodeString copies a Go string and returns it as a nstring. -// The result is always a copy, because go_seq_to_objc_string -// uses NSString initWithBytesNoCopy to another copy. -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,10 +44,9 @@ func encodeString(s string, cpy bool) C.nstring { return C.nstring{ptr: ptr, len: n} } -// decodeString converts a nstring to a Go string. -// The data in str is always a copy, so cpy is ignored -// and the data is always freed. -func decodeString(str C.nstring, cpy bool) string { +// decodeString converts a nstring to a Go string. The +// data in str is freed after use. +func decodeString(str C.nstring) string { if str.ptr == nil { return "" } diff --git a/bind/seq/string.go b/bind/seq/string.go index a39b115..b578d55 100644 --- a/bind/seq/string.go +++ b/bind/seq/string.go @@ -4,10 +4,7 @@ package seq -import ( - "unicode/utf16" - "unsafe" -) +import "unicode/utf16" // Based heavily on package unicode/utf16 from the Go standard library. @@ -27,10 +24,6 @@ const ( surrSelf = 0x10000 ) -func writeUint16(b []byte, v rune) { - *(*uint16)(unsafe.Pointer(&b[0])) = uint16(v) -} - // UTF16Encode utf16 encodes s into chars. It returns the resulting // length in units of uint16. It is assumed that the chars slice // has enough room for the encoded string. diff --git a/bind/testdata/basictypes.go.golden b/bind/testdata/basictypes.go.golden index 89e6be7..a3eb955 100644 --- a/bind/testdata/basictypes.go.golden +++ b/bind/testdata/basictypes.go.golden @@ -49,7 +49,7 @@ func proxybasictypes__Error() C.nstring { } else { _res_0_str = res_0.Error() } - _res_0 := encodeString(_res_0_str, true) + _res_0 := encodeString(_res_0_str) return _res_0 } @@ -63,7 +63,7 @@ func proxybasictypes__ErrorPair() (C.nint, C.nstring) { } else { _res_1_str = res_1.Error() } - _res_1 := encodeString(_res_1_str, true) + _res_1 := encodeString(_res_1_str) return _res_0, _res_1 } diff --git a/bind/testdata/interfaces.go.golden b/bind/testdata/interfaces.go.golden index f55a960..4c53958 100644 --- a/bind/testdata/interfaces.go.golden +++ b/bind/testdata/interfaces.go.golden @@ -32,7 +32,7 @@ func proxyinterfaces_Error_Err(refnum C.int32_t) C.nstring { } else { _res_0_str = res_0.Error() } - _res_0 := encodeString(_res_0_str, true) + _res_0 := encodeString(_res_0_str) return _res_0 } @@ -40,7 +40,7 @@ type proxyinterfaces_Error _seq.Ref func (p *proxyinterfaces_Error) Err() error { res := C.cproxyinterfaces_Error_Err(C.int32_t(p.Num)) - _res_str := decodeString(res, true) + _res_str := decodeString(res) _res := toError(_res_str) return _res } @@ -154,7 +154,7 @@ func proxyinterfaces__CallErr(param_e C.int32_t) C.nstring { } else { _res_0_str = res_0.Error() } - _res_0 := encodeString(_res_0_str, true) + _res_0 := encodeString(_res_0_str) return _res_0 } diff --git a/bind/testdata/interfaces.java.c.golden b/bind/testdata/interfaces.java.c.golden index ba36f77..202c1fe 100644 --- a/bind/testdata/interfaces.java.c.golden +++ b/bind/testdata/interfaces.java.c.golden @@ -108,7 +108,7 @@ nstring cproxyinterfaces_Error_Err(int32_t refnum) { jobject o = go_seq_from_refnum(env, refnum, proxy_class_interfaces_Error, proxy_class_interfaces_Error_cons); (*env)->CallVoidMethod(env, o, mid_Error_Err); jstring exc = go_seq_get_exception_message(env); - nstring _exc = go_seq_from_java_string(env, exc, 1); + nstring _exc = go_seq_from_java_string(env, exc); go_seq_pop_local_frame(env); return _exc; } diff --git a/bind/testdata/issue10788.go.golden b/bind/testdata/issue10788.go.golden index 74e7d65..0981a16 100644 --- a/bind/testdata/issue10788.go.golden +++ b/bind/testdata/issue10788.go.golden @@ -26,7 +26,7 @@ type proxyTestStruct _seq.Ref //export proxyissue10788_TestStruct_Value_Set func proxyissue10788_TestStruct_Value_Set(refnum C.int32_t, v C.nstring) { ref := _seq.FromRefNum(int32(refnum)) - _v := decodeString(v, false) + _v := decodeString(v) ref.Get().(*issue10788.TestStruct).Value = _v } @@ -34,7 +34,7 @@ func proxyissue10788_TestStruct_Value_Set(refnum C.int32_t, v C.nstring) { func proxyissue10788_TestStruct_Value_Get(refnum C.int32_t) C.nstring { ref := _seq.FromRefNum(int32(refnum)) v := ref.Get().(*issue10788.TestStruct).Value - _v := encodeString(v, true) + _v := encodeString(v) return _v } @@ -53,7 +53,7 @@ func proxyissue10788_TestInterface_MultipleUnnamedParams(refnum C.int32_t, param ref := _seq.FromRefNum(int32(refnum)) v := ref.Get().(issue10788.TestInterface) _param_p0 := int(param_p0) - _param_p1 := decodeString(param_p1, false) + _param_p1 := decodeString(param_p1) _param_p2 := int64(param_p2) v.MultipleUnnamedParams(_param_p0, _param_p1, _param_p2) } @@ -70,7 +70,7 @@ func (p *proxyissue10788_TestInterface) DoSomeWork(param_s *issue10788.TestStruc func (p *proxyissue10788_TestInterface) MultipleUnnamedParams(param_p0 int, param_p1 string, param_p2 int64) { _param_p0 := C.nint(param_p0) - _param_p1 := encodeString(param_p1, false) + _param_p1 := encodeString(param_p1) _param_p2 := C.int64_t(param_p2) C.cproxyissue10788_TestInterface_MultipleUnnamedParams(C.int32_t(p.Num), _param_p0, _param_p1, _param_p2) } diff --git a/bind/testdata/issue10788.java.c.golden b/bind/testdata/issue10788.java.c.golden index 13da27c..9d63edc 100644 --- a/bind/testdata/issue10788.java.c.golden +++ b/bind/testdata/issue10788.java.c.golden @@ -35,11 +35,8 @@ Java_go_issue10788_Issue10788_init(JNIEnv *env, jclass _unused) { JNIEXPORT void JNICALL Java_go_issue10788_Issue10788_00024TestStruct_setValue(JNIEnv *env, jobject this, jstring v) { int32_t o = go_seq_to_refnum(env, this); - nstring _v = go_seq_from_java_string(env, v, 0); + nstring _v = go_seq_from_java_string(env, v); proxyissue10788_TestStruct_Value_Set(o, _v); - if (_v.chars != NULL) { - (*env)->ReleaseStringChars(env, v, _v.chars); - } } JNIEXPORT jstring JNICALL @@ -69,12 +66,9 @@ JNIEXPORT void JNICALL Java_go_issue10788_Issue10788_00024TestInterface_00024Proxy_MultipleUnnamedParams(JNIEnv* env, jobject this, jlong p0, jstring p1, jlong p2) { int32_t o = go_seq_to_refnum(env, this); nint _p0 = (nint)p0; - nstring _p1 = go_seq_from_java_string(env, p1, 0); + nstring _p1 = go_seq_from_java_string(env, p1); int64_t _p2 = (int64_t)p2; proxyissue10788_TestInterface_MultipleUnnamedParams(o, _p0, _p1, _p2); - if (_p1.chars != NULL) { - (*env)->ReleaseStringChars(env, p1, _p1.chars); - } } void cproxyissue10788_TestInterface_MultipleUnnamedParams(int32_t refnum, nint p0, nstring p1, int64_t p2) { diff --git a/bind/testdata/issue12328.go.golden b/bind/testdata/issue12328.go.golden index fbcf4fa..65ce4c8 100644 --- a/bind/testdata/issue12328.go.golden +++ b/bind/testdata/issue12328.go.golden @@ -26,7 +26,7 @@ type proxyT _seq.Ref //export proxyissue12328_T_Err_Set func proxyissue12328_T_Err_Set(refnum C.int32_t, v C.nstring) { ref := _seq.FromRefNum(int32(refnum)) - _v_str := decodeString(v, false) + _v_str := decodeString(v) _v := toError(_v_str) ref.Get().(*issue12328.T).Err = _v } @@ -41,6 +41,6 @@ func proxyissue12328_T_Err_Get(refnum C.int32_t) C.nstring { } else { _v_str = v.Error() } - _v := encodeString(_v_str, true) + _v := encodeString(_v_str) return _v } diff --git a/bind/testdata/issue12328.java.c.golden b/bind/testdata/issue12328.java.c.golden index 7fb843f..8aca32a 100644 --- a/bind/testdata/issue12328.java.c.golden +++ b/bind/testdata/issue12328.java.c.golden @@ -24,11 +24,8 @@ Java_go_issue12328_Issue12328_init(JNIEnv *env, jclass _unused) { JNIEXPORT void JNICALL Java_go_issue12328_Issue12328_00024T_setErr(JNIEnv *env, jobject this, jstring v) { int32_t o = go_seq_to_refnum(env, this); - nstring _v = go_seq_from_java_string(env, v, 0); + nstring _v = go_seq_from_java_string(env, v); proxyissue12328_T_Err_Set(o, _v); - if (_v.chars != NULL) { - (*env)->ReleaseStringChars(env, v, _v.chars); - } } JNIEXPORT jstring JNICALL diff --git a/bind/testdata/issue12403.go.golden b/bind/testdata/issue12403.go.golden index 585ee18..d0d3c6e 100644 --- a/bind/testdata/issue12403.go.golden +++ b/bind/testdata/issue12403.go.golden @@ -25,9 +25,9 @@ var _ = _seq.FromRefNum func proxyissue12403_Parsable_FromJSON(refnum C.int32_t, param_jstr C.nstring) C.nstring { ref := _seq.FromRefNum(int32(refnum)) v := ref.Get().(issue12403.Parsable) - _param_jstr := decodeString(param_jstr, false) + _param_jstr := decodeString(param_jstr) res_0 := v.FromJSON(_param_jstr) - _res_0 := encodeString(res_0, true) + _res_0 := encodeString(res_0) return _res_0 } @@ -36,30 +36,30 @@ func proxyissue12403_Parsable_ToJSON(refnum C.int32_t) (C.nstring, C.nstring) { ref := _seq.FromRefNum(int32(refnum)) v := ref.Get().(issue12403.Parsable) res_0, res_1 := v.ToJSON() - _res_0 := encodeString(res_0, true) + _res_0 := encodeString(res_0) var _res_1_str string if res_1 == nil { _res_1_str = "" } else { _res_1_str = res_1.Error() } - _res_1 := encodeString(_res_1_str, true) + _res_1 := encodeString(_res_1_str) return _res_0, _res_1 } type proxyissue12403_Parsable _seq.Ref func (p *proxyissue12403_Parsable) FromJSON(param_jstr string) string { - _param_jstr := encodeString(param_jstr, false) + _param_jstr := encodeString(param_jstr) res := C.cproxyissue12403_Parsable_FromJSON(C.int32_t(p.Num), _param_jstr) - _res := decodeString(res, true) + _res := decodeString(res) return _res } func (p *proxyissue12403_Parsable) ToJSON() (string, error) { res := C.cproxyissue12403_Parsable_ToJSON(C.int32_t(p.Num)) - res_0 := decodeString(res.r0, true) - res_1_str := decodeString(res.r1, true) + res_0 := decodeString(res.r0) + res_1_str := decodeString(res.r1) res_1 := toError(res_1_str) return res_0, res_1 } diff --git a/bind/testdata/issue12403.java.c.golden b/bind/testdata/issue12403.java.c.golden index 511180f..b8d9e98 100644 --- a/bind/testdata/issue12403.java.c.golden +++ b/bind/testdata/issue12403.java.c.golden @@ -30,11 +30,8 @@ Java_go_issue12403_Issue12403_init(JNIEnv *env, jclass _unused) { JNIEXPORT jstring JNICALL Java_go_issue12403_Issue12403_00024Parsable_00024Proxy_FromJSON(JNIEnv* env, jobject this, jstring jstr) { int32_t o = go_seq_to_refnum(env, this); - nstring _jstr = go_seq_from_java_string(env, jstr, 0); + nstring _jstr = go_seq_from_java_string(env, jstr); nstring r0 = proxyissue12403_Parsable_FromJSON(o, _jstr); - if (_jstr.chars != NULL) { - (*env)->ReleaseStringChars(env, jstr, _jstr.chars); - } jstring _r0 = go_seq_to_java_string(env, r0); return _r0; } @@ -44,7 +41,7 @@ nstring cproxyissue12403_Parsable_FromJSON(int32_t refnum, nstring jstr) { jobject o = go_seq_from_refnum(env, refnum, proxy_class_issue12403_Parsable, proxy_class_issue12403_Parsable_cons); jstring _jstr = go_seq_to_java_string(env, jstr); jstring res = (*env)->CallObjectMethod(env, o, mid_Parsable_FromJSON, _jstr); - nstring _res = go_seq_from_java_string(env, res, 1); + nstring _res = go_seq_from_java_string(env, res); go_seq_pop_local_frame(env); return _res; } @@ -63,9 +60,9 @@ struct cproxyissue12403_Parsable_ToJSON_return cproxyissue12403_Parsable_ToJSON( JNIEnv *env = go_seq_push_local_frame(10); jobject o = go_seq_from_refnum(env, refnum, proxy_class_issue12403_Parsable, proxy_class_issue12403_Parsable_cons); jstring res = (*env)->CallObjectMethod(env, o, mid_Parsable_ToJSON); - nstring _res = go_seq_from_java_string(env, res, 1); + nstring _res = go_seq_from_java_string(env, res); jstring exc = go_seq_get_exception_message(env); - nstring _exc = go_seq_from_java_string(env, exc, 1); + nstring _exc = go_seq_from_java_string(env, exc); cproxyissue12403_Parsable_ToJSON_return sres = { _res, _exc }; diff --git a/bind/testdata/structs.go.golden b/bind/testdata/structs.go.golden index 69cc85b..4ec9ca0 100644 --- a/bind/testdata/structs.go.golden +++ b/bind/testdata/structs.go.golden @@ -68,7 +68,7 @@ func proxystructs_S_Identity(refnum C.int32_t) (C.int32_t, C.nstring) { } else { _res_1_str = res_1.Error() } - _res_1 := encodeString(_res_1_str, true) + _res_1 := encodeString(_res_1_str) return _res_0, _res_1 } @@ -132,6 +132,6 @@ func proxystructs__IdentityWithError(param_s C.int32_t) (C.int32_t, C.nstring) { } else { _res_1_str = res_1.Error() } - _res_1 := encodeString(_res_1_str, true) + _res_1 := encodeString(_res_1_str) return _res_0, _res_1 } diff --git a/bind/testdata/try.go.golden b/bind/testdata/try.go.golden index e1df038..2dfe8e4 100644 --- a/bind/testdata/try.go.golden +++ b/bind/testdata/try.go.golden @@ -24,6 +24,6 @@ var _ = _seq.FromRefNum //export proxytry__This func proxytry__This() C.nstring { res_0 := try.This() - _res_0 := encodeString(res_0, true) + _res_0 := encodeString(res_0) return _res_0 } diff --git a/bind/testdata/vars.go.golden b/bind/testdata/vars.go.golden index 5ef1baf..0f9c16e 100644 --- a/bind/testdata/vars.go.golden +++ b/bind/testdata/vars.go.golden @@ -82,14 +82,14 @@ func var_getvars_AFloat64() C.double { //export var_setvars_AString func var_setvars_AString(v C.nstring) { - _v := decodeString(v, false) + _v := decodeString(v) vars.AString = _v } //export var_getvars_AString func var_getvars_AString() C.nstring { v := vars.AString - _v := encodeString(v, true) + _v := encodeString(v) return _v } diff --git a/bind/testdata/vars.java.c.golden b/bind/testdata/vars.java.c.golden index dedc189..7bbda6d 100644 --- a/bind/testdata/vars.java.c.golden +++ b/bind/testdata/vars.java.c.golden @@ -82,11 +82,8 @@ Java_go_vars_Vars_getAFloat64(JNIEnv *env, jclass clazz) { JNIEXPORT void JNICALL Java_go_vars_Vars_setAString(JNIEnv *env, jclass clazz, jstring v) { - nstring _v = go_seq_from_java_string(env, v, 0); + nstring _v = go_seq_from_java_string(env, v); var_setvars_AString(_v); - if (_v.chars != NULL) { - (*env)->ReleaseStringChars(env, v, _v.chars); - } } JNIEXPORT jstring JNICALL