Skip to content

Commit

Permalink
Generalize C#/JS string interning and ensure most bindings code suppo…
Browse files Browse the repository at this point in the history 
…rts interned strings (dotnet#54086)

This PR refactors all C# -> JS string decoding to go through a single path that maintains an intern table
  • Loading branch information
@kg
kg authored Jun 15, 2021
1 parent ce95c98 commit 2506a74
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 53 deletions.
49 changes: 21 additions & 28 deletions src/mono/wasm/runtime/binding_support.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
},

Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 + "'");
}
Expand Down Expand Up @@ -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 + "'");
Expand Down Expand Up @@ -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 + "'");
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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");
Expand Down
52 changes: 33 additions & 19 deletions src/mono/wasm/runtime/driver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
51 changes: 45 additions & 6 deletions src/mono/wasm/runtime/library_mono.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 2506a74

Please sign in to comment.