From: Katelyn Gadd Date: Tue, 15 Jun 2021 20:38:10 +0000 (-0700) Subject: Generalize C#/JS string interning and ensure most bindings code supports interned... X-Git-Tag: submit/tizen/20210909.063632~750 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2506a7423d59a09c17a6b4d766845bd1b64aa11a;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Generalize C#/JS string interning and ensure most bindings code supports interned strings (#54086) This PR refactors all C# -> JS string decoding to go through a single path that maintains an intern table --- diff --git a/src/mono/wasm/runtime/binding_support.js b/src/mono/wasm/runtime/binding_support.js index 6661228..6f0040f 100644 --- a/src/mono/wasm/runtime/binding_support.js +++ b/src/mono/wasm/runtime/binding_support.js @@ -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"); diff --git a/src/mono/wasm/runtime/driver.c b/src/mono/wasm/runtime/driver.c index c0ab7bd..ca98e34 100644 --- a/src/mono/wasm/runtime/driver.c +++ b/src/mono/wasm/runtime/driver.c @@ -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; +} diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 685aaaf..1186660 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -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) {