Generalize C#/JS string interning and ensure most bindings code supports interned...
authorKatelyn Gadd <kg@luminance.org>
Tue, 15 Jun 2021 20:38:10 +0000 (13:38 -0700)
committerGitHub <noreply@github.com>
Tue, 15 Jun 2021 20:38:10 +0000 (13:38 -0700)
This PR refactors all C# -> JS string decoding to go through a single path that maintains an intern table

src/mono/wasm/runtime/binding_support.js
src/mono/wasm/runtime/driver.c
src/mono/wasm/runtime/library_mono.js

index 6661228..6f0040f 100644 (file)
@@ -154,8 +154,7 @@ var BindingSupportLib = {
                        this._interned_string_full_root_buffers = [];
                        this._interned_string_current_root_buffer = null;
                        this._interned_string_current_root_buffer_count = 0;
-                       this._interned_string_table = new Map ();
-                       this._managed_pointer_to_interned_string_table = new Map ();
+                       this._interned_js_string_table = new Map ();
                },
 
                // Ensures the string is already interned on both the managed and JavaScript sides,
@@ -165,7 +164,7 @@ var BindingSupportLib = {
                                return this._empty_string;
 
                        var ptr = this.js_string_to_mono_string_interned (string);
-                       var result = this._managed_pointer_to_interned_string_table.get (ptr);
+                       var result = MONO.interned_string_table.get (ptr);
                        return result;
                },
 
@@ -199,8 +198,10 @@ var BindingSupportLib = {
                        if (!ptr)
                                throw new Error ("mono_wasm_intern_string produced a null pointer");
 
-                       this._interned_string_table.set (string, ptr);
-                       this._managed_pointer_to_interned_string_table.set (ptr, string);
+                       this._interned_js_string_table.set (string, ptr);
+                       if (!MONO.interned_string_table)
+                               MONO.interned_string_table = new Map();
+                       MONO.interned_string_table.set (ptr, string);
 
                        if ((string.length === 0) && !this._empty_string_ptr)
                                this._empty_string_ptr = ptr;
@@ -216,7 +217,7 @@ var BindingSupportLib = {
                        if ((text.length === 0) && this._empty_string_ptr)
                                return this._empty_string_ptr;
 
-                       var ptr = this._interned_string_table.get (string);
+                       var ptr = this._interned_js_string_table.get (string);
                        if (ptr)
                                return ptr;
 
@@ -243,7 +244,7 @@ var BindingSupportLib = {
                        //  very expensive. Because we can not guarantee it won't happen, try to minimize
                        //  the cost of this and prevent performance issues for large strings
                        if (string.length <= 256) {
-                               var interned = this._interned_string_table.get (string);
+                               var interned = this._interned_js_string_table.get (string);
                                if (interned)
                                        return interned;
                        }
@@ -279,20 +280,13 @@ var BindingSupportLib = {
                },
 
                _get_string_from_intern_table: function (mono_obj) {
-                       return this._managed_pointer_to_interned_string_table.get (mono_obj);
+                       if (!MONO.interned_string_table)
+                               return undefined;
+                       return MONO.interned_string_table.get (mono_obj);
                },
 
-               conv_string: function (mono_obj, interned) {
-                       var interned_instance = this._get_string_from_intern_table(mono_obj);
-                       if (interned_instance !== undefined)
-                               return interned_instance;
-
-                       var result = MONO.string_decoder.copy (mono_obj);
-                       if (interned) {
-                               // This string is interned on the managed side but we didn't have it in our cache.
-                               this._store_string_in_intern_table (result, mono_obj, false);
-                       }
-                       return result;
+               conv_string: function (mono_obj) {
+                       return MONO.string_decoder.copy (mono_obj);
                },
 
                is_nested_array: function (ele) {
@@ -419,9 +413,8 @@ var BindingSupportLib = {
                                        // TODO: Fix this once emscripten offers HEAPI64/HEAPU64 or can return them
                                        throw new Error ("int64 not available");
                                case 3: // string
-                                       return this.conv_string (mono_obj, false);
                                case 29: // interned string
-                                       return this.conv_string (mono_obj, true);
+                                       return this.conv_string (mono_obj);
                                case 4: //vts
                                        throw new Error ("no idea on how to unbox value types");
                                case 5: // delegate
@@ -1205,7 +1198,7 @@ var BindingSupportLib = {
                        if (exception === 0)
                                return null;
 
-                       var msg = this.conv_string (result, false);
+                       var msg = this.conv_string (result);
                        var err = new Error (msg); //the convention is that invoke_method ToString () any outgoing exception
                        // console.warn ("error", msg, "at location", err.stack);
                        return err;
@@ -1695,8 +1688,8 @@ var BindingSupportLib = {
                        return BINDING.js_string_to_mono_string ("Invalid JS object handle '" + js_handle + "'");
                }
 
-               var js_name = BINDING.conv_string (method_name, false);
-               if (!js_name) {
+               var js_name = BINDING.unbox_mono_obj (method_name);
+               if (!js_name || (typeof(js_name) !== "string")) {
                        setValue (is_exception, 1, "i32");
                        return BINDING.js_string_to_mono_string ("Invalid method name object '" + method_name + "'");
                }
@@ -1729,7 +1722,7 @@ var BindingSupportLib = {
                        return BINDING.js_string_to_mono_string ("Invalid JS object handle '" + js_handle + "'");
                }
 
-               var js_name = BINDING.conv_string (property_name, false);
+               var js_name = BINDING.conv_string (property_name);
                if (!js_name) {
                        setValue (is_exception, 1, "i32");
                        return BINDING.js_string_to_mono_string ("Invalid property name object '" + js_name + "'");
@@ -1760,7 +1753,7 @@ var BindingSupportLib = {
                        return BINDING.js_string_to_mono_string ("Invalid JS object handle '" + js_handle + "'");
                }
 
-               var property = BINDING.conv_string (property_name, false);
+               var property = BINDING.conv_string (property_name);
                if (!property) {
                        setValue (is_exception, 1, "i32");
                        return BINDING.js_string_to_mono_string ("Invalid property name object '" + property_name + "'");
@@ -1844,7 +1837,7 @@ var BindingSupportLib = {
        mono_wasm_get_global_object: function(global_name, is_exception) {
                BINDING.bindings_lazy_init ();
 
-               var js_name = BINDING.conv_string (global_name, false);
+               var js_name = BINDING.conv_string (global_name);
 
                var globalObj;
 
@@ -1902,7 +1895,7 @@ var BindingSupportLib = {
        mono_wasm_new: function (core_name, args, is_exception) {
                BINDING.bindings_lazy_init ();
 
-               var js_name = BINDING.conv_string (core_name, false);
+               var js_name = BINDING.conv_string (core_name);
 
                if (!js_name) {
                        setValue (is_exception, 1, "i32");
index c0ab7bd..ca98e34 100644 (file)
@@ -98,29 +98,19 @@ mono_wasm_invoke_js (MonoString *str, int *is_exception)
        if (str == NULL)
                return NULL;
 
-       int is_interned = mono_string_instance_is_interned (str);
-       mono_unichar2 *native_chars = mono_string_chars (str);
-       int native_len = mono_string_length (str) * 2;
-       int native_res_len;
+       int native_res_len = 0;
        int *p_native_res_len = &native_res_len;
 
        mono_unichar2 *native_res = (mono_unichar2*)EM_ASM_INT ({
-               var str;
-               // If the expression is interned, use binding_support's intern table implementation to
-               //  avoid decoding it again unless necessary
-               // We could technically use conv_string for both cases here, but it's more expensive
-               //  than using decode directly in the case where the expression isn't interned
-               if ($4)
-                       str = BINDING.conv_string($5, true);
-               else
-                       str = MONO.string_decoder.decode ($0, $0 + $1);
+               var js_str = MONO.string_decoder.copy ($0);
 
                try {
-                       var res = eval (str);
-                       if (res === null || res == undefined)
-                               return 0;
-                       res = res.toString ();
+                       var res = eval (js_str);
                        setValue ($2, 0, "i32");
+                       if (res === null || res === undefined)
+                               return 0;
+                       else
+                               res = res.toString ();
                } catch (e) {
                        res = e.toString();
                        setValue ($2, 1, "i32");
@@ -139,9 +129,9 @@ mono_wasm_invoke_js (MonoString *str, int *is_exception)
                }
                var buff = Module._malloc((res.length + 1) * 2);
                stringToUTF16 (res, buff, (res.length + 1) * 2);
-               setValue ($3, res.length, "i32");
+               setValue ($1, res.length, "i32");
                return buff;
-       }, (int)native_chars, native_len, is_exception, p_native_res_len, is_interned, (int)str);
+       }, (int)str, p_native_res_len, is_exception);
 
        if (native_res == NULL)
                return NULL;
@@ -731,6 +721,7 @@ mono_wasm_string_get_utf8 (MonoString *str)
        return mono_string_to_utf8 (str); //XXX JS is responsible for freeing this
 }
 
+// Deprecated
 EMSCRIPTEN_KEEPALIVE void
 mono_wasm_string_convert (MonoString *str)
 {
@@ -1132,3 +1123,26 @@ mono_wasm_intern_string (MonoString *string)
 {
        return mono_string_intern (string);
 }
+
+EMSCRIPTEN_KEEPALIVE void
+mono_wasm_string_get_data (
+       MonoString *string, mono_unichar2 **outChars, int *outLengthBytes, int *outIsInterned
+) {
+       if (!string) {
+               if (outChars)
+                       *outChars = 0;
+               if (outLengthBytes)
+                       *outLengthBytes = 0;
+               if (outIsInterned)
+                       *outIsInterned = 1;
+               return;
+       }
+
+       if (outChars)
+               *outChars = mono_string_chars (string);
+       if (outLengthBytes)
+               *outLengthBytes = mono_string_length (string) * 2;
+       if (outIsInterned)
+               *outIsInterned = mono_string_instance_is_interned (string);
+       return;
+}
index 685aaaf..1186660 100644 (file)
@@ -487,15 +487,54 @@ var MonoSupportLib = {
                mono_text_decoder: undefined,
                string_decoder: {
                        copy: function (mono_string) {
-                               if (mono_string == 0)
+                               if (mono_string === 0)
                                        return null;
 
-                               if (!this.mono_wasm_string_convert)
-                                       this.mono_wasm_string_convert = Module.cwrap ("mono_wasm_string_convert", null, ['number']);
+                               if (!this.mono_wasm_string_root)
+                                       this.mono_wasm_string_root = MONO.mono_wasm_new_root ();
+                               this.mono_wasm_string_root.value = mono_string;
+
+                               if (!this.mono_wasm_string_get_data)
+                                       this.mono_wasm_string_get_data = Module.cwrap ("mono_wasm_string_get_data", null, ['number', 'number', 'number', 'number']);
+                               
+                               if (!this.mono_wasm_string_decoder_buffer)
+                                       this.mono_wasm_string_decoder_buffer = Module._malloc(12);
+                               
+                               let ppChars = this.mono_wasm_string_decoder_buffer + 0,
+                                       pLengthBytes = this.mono_wasm_string_decoder_buffer + 4,
+                                       pIsInterned = this.mono_wasm_string_decoder_buffer + 8;
+                               
+                               this.mono_wasm_string_get_data (mono_string, ppChars, pLengthBytes, pIsInterned);
+
+                               // TODO: Is this necessary?
+                               if (!this.mono_wasm_empty_string)
+                                       this.mono_wasm_empty_string = "";
+
+                               let result = this.mono_wasm_empty_string;
+                               let lengthBytes = Module.HEAP32[pLengthBytes / 4],
+                                       pChars = Module.HEAP32[ppChars / 4],
+                                       isInterned = Module.HEAP32[pIsInterned / 4];
+
+                               if (pLengthBytes && pChars) {
+                                       if (
+                                               isInterned && 
+                                               MONO.interned_string_table && 
+                                               MONO.interned_string_table.has(mono_string)
+                                       ) {
+                                               result = MONO.interned_string_table.get(mono_string);
+                                               // console.log("intern table cache hit", mono_string, result.length);
+                                       } else {
+                                               result = this.decode(pChars, pChars + lengthBytes, false);
+                                               if (isInterned) {
+                                                       if (!MONO.interned_string_table)
+                                                               MONO.interned_string_table = new Map();
+                                                       // console.log("interned", mono_string, result.length);
+                                                       MONO.interned_string_table.set(mono_string, result);
+                                               }
+                                       }                                               
+                               }
 
-                               this.mono_wasm_string_convert (mono_string);
-                               var result = this.result;
-                               this.result = undefined;
+                               this.mono_wasm_string_root.value = 0;
                                return result;
                        },
                        decode: function (start, end, save) {