embind: Use single type object for emval. (#20806)

If register_type is used, emval will be registered multiple times for different type id's, but only a single type object is needed on the JS side for all of them.
emscripten-core · Jan 18, 2024 · e6415d6 · e6415d6
1 parent 946c850
commit e6415d6
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 32 deletions.
diff --git a/src/embind/embind.js b/src/embind/embind.js
@@ -38,7 +38,25 @@ var LibraryEmbind = {
 #if EMBIND_AOT
   $InvokerFunctions: '<<< EMBIND_AOT_OUTPUT >>>',
 #endif
-
+  // If register_type is used, emval will be registered multiple times for
+  // different type id's, but only a single type object is needed on the JS side
+  // for all of them. Store the type for reuse.
+  $EmValType__deps: ['_emval_decref', '$Emval', '$simpleReadValueFromPointer', '$GenericWireTypeSize'],
+  $EmValType: `{
+    name: 'emscripten::val',
+    'fromWireType': (handle) => {
+      var rv = Emval.toValue(handle);
+      __emval_decref(handle);
+      return rv;
+    },
+    'toWireType': (destructors, value) => Emval.toHandle(value),
+    'argPackAdvance': GenericWireTypeSize,
+    'readValueFromPointer': simpleReadValueFromPointer,
+    destructorFunction: null, // This type does not need a destructor
+
+    // TODO: do we need a deleteObject here?  write a test where
+    // emval is passed into JS via an interface
+  }`,
   $init_embind__deps: [
     '$getInheritedInstanceCount', '$getLiveInheritedInstances',
     '$flushPendingDeletes', '$setDelayFunction'],
@@ -668,32 +686,17 @@ var LibraryEmbind = {
   },
 
   _embind_register_emval__deps: [
-    '_emval_decref', '$Emval',
-    '$readLatin1String', '$registerType', '$simpleReadValueFromPointer'],
-  _embind_register_emval: (rawType, name) => {
-    name = readLatin1String(name);
-    registerType(rawType, {
-      name,
-      'fromWireType': (handle) => {
-        var rv = Emval.toValue(handle);
-        __emval_decref(handle);
-        return rv;
-      },
-      'toWireType': (destructors, value) => Emval.toHandle(value),
-      'argPackAdvance': GenericWireTypeSize,
-      'readValueFromPointer': simpleReadValueFromPointer,
-      destructorFunction: null, // This type does not need a destructor
-    });
-  },
+    '$registerType',  '$EmValType'],
+  _embind_register_emval: (rawType) => registerType(rawType, EmValType),
 
   _embind_register_user_type__deps: ['_embind_register_emval'],
   _embind_register_user_type: (rawType, name) => {
-    __embind_register_emval(rawType, name);
+    __embind_register_emval(rawType);
   },
 
   _embind_register_optional__deps: ['_embind_register_emval'],
   _embind_register_optional: (rawOptionalType, rawType) => {
-    __embind_register_emval(rawOptionalType, "");
+    __embind_register_emval(rawOptionalType);
   },
 
   _embind_register_memory_view__deps: ['$readLatin1String', '$registerType'],

diff --git a/src/embind/embind_gen.js b/src/embind/embind_gen.js
@@ -463,8 +463,9 @@ var LibraryEmbind = {
   _embind_register_std_wstring: (rawType, charSize, name) => {
     registerPrimitiveType(rawType, name, 'function');
   },
-  _embind_register_emval: (rawType, name) => {
-    registerPrimitiveType(rawType, name, 'none');
+  _embind_register_emval__deps: ['$registerType', '$PrimitiveType'],
+  _embind_register_emval: (rawType) => {
+    registerType(rawType, new PrimitiveType(rawType, 'emscripten::val', 'none'));
   },
   _embind_register_user_type__deps: ['$registerType', '$readLatin1String', '$UserType'],
   _embind_register_user_type: (rawType, name) => {

diff --git a/src/library_sigs.js b/src/library_sigs.js
@@ -297,7 +297,7 @@ sigs = {
   _embind_register_class_function__sig: 'vppippppii',
   _embind_register_class_property__sig: 'vpppppppppp',
   _embind_register_constant__sig: 'vppd',
-  _embind_register_emval__sig: 'vpp',
+  _embind_register_emval__sig: 'vp',
   _embind_register_enum__sig: 'vpppi',
   _embind_register_enum_value__sig: 'vppi',
   _embind_register_float__sig: 'vppp',

diff --git a/system/include/emscripten/bind.h b/system/include/emscripten/bind.h
@@ -91,8 +91,7 @@ void _embind_register_std_wstring(
     const char* name);
 
 void _embind_register_emval(
-    TYPEID emvalType,
-    const char* name);
+    TYPEID emvalType);
 
 void _embind_register_memory_view(
     TYPEID memoryViewType,

diff --git a/system/lib/embind/bind.cpp b/system/lib/embind/bind.cpp
@@ -154,7 +154,7 @@ EMSCRIPTEN_BINDINGS(builtin) {
   _embind_register_std_wstring(TypeID<std::wstring>::get(), sizeof(wchar_t), "std::wstring");
   _embind_register_std_wstring(TypeID<std::u16string>::get(), sizeof(char16_t), "std::u16string");
   _embind_register_std_wstring(TypeID<std::u32string>::get(), sizeof(char32_t), "std::u32string");
-  _embind_register_emval(TypeID<val>::get(), "emscripten::val");
+  _embind_register_emval(TypeID<val>::get());
 
   // Some of these types are aliases for each other. Luckily,
   // embind.js's _embind_register_memory_view ignores duplicate

diff --git a/test/code_size/embind_val_wasm.json b/test/code_size/embind_val_wasm.json
@@ -1,10 +1,10 @@
 {
   "a.html": 673,
   "a.html.gz": 431,
-  "a.js": 7395,
-  "a.js.gz": 3109,
-  "a.wasm": 11458,
-  "a.wasm.gz": 5733,
-  "total": 19526,
-  "total_gz": 9273
+  "a.js": 7411,
+  "a.js.gz": 3140,
+  "a.wasm": 11439,
+  "a.wasm.gz": 5729,
+  "total": 19523,
+  "total_gz": 9300
 }