[emval] Move reference counting to C++

src/embind/embind.js

@@ -659,17 +659,13 @@ var LibraryEmbind = {
   },
 
   _embind_register_emval__deps: [
-    '_emval_decref', '$Emval',
+    '$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;
-      },
+      'fromWireType': (handle) => Emval.toValue(handle),
       'toWireType': (destructors, value) => Emval.toHandle(value),
       'argPackAdvance': GenericWireTypeSize,
       'readValueFromPointer': simpleReadValueFromPointer,

src/embind/emval.js

@@ -27,10 +27,10 @@ var LibraryEmVal = {
     // reserve some special values. These never get de-allocated.
     // The HandleAllocator takes care of reserving zero.
     emval_handles.allocated.push(
-      {value: undefined},
-      {value: null},
-      {value: true},
-      {value: false},
+      undefined,
+      null,
+      true,
+      false,
     );
     emval_handles.reserved = emval_handles.allocated.length
     Module['count_emval_handles'] = count_emval_handles;
@@ -63,12 +63,7 @@ var LibraryEmVal = {
 
   $Emval__deps: ['$emval_handles', '$throwBindingError', '$init_emval'],
   $Emval: {
-    toValue: (handle) => {
-      if (!handle) {
-          throwBindingError('Cannot use deleted val. handle = ' + handle);
-      }
-      return emval_handles.get(handle).value;
-    },
+    toValue: (handle) => emval_handles.get(handle),
 
     toHandle: (value) => {
       switch (value) {
@@ -77,31 +72,24 @@ var LibraryEmVal = {
         case true: return 3;
         case false: return 4;
         default:{
-          return emval_handles.allocate({refcount: 1, value: value});
+          return emval_handles.allocate(value);
         }
       }
     }
   },
 
-  _emval_incref__deps: ['$emval_handles'],
-  _emval_incref: (handle) => {
-    if (handle > 4) {
-      emval_handles.get(handle).refcount += 1;
-    }
-  },
-
-  _emval_decref__deps: ['$emval_handles'],
-  _emval_decref: (handle) => {
-    if (handle >= emval_handles.reserved && 0 === --emval_handles.get(handle).refcount) {
+  _emval_free__deps: ['$emval_handles'],
+  _emval_free: (handle) => {
+    if (handle >= emval_handles.reserved) {
       emval_handles.free(handle);
     }
   },
 
-  _emval_run_destructors__deps: ['_emval_decref', '$Emval', '$runDestructors'],
+  _emval_run_destructors__deps: ['_emval_free', '$Emval', '$runDestructors'],
   _emval_run_destructors: (handle) => {
     var destructors = Emval.toValue(handle);
     runDestructors(destructors);
-    __emval_decref(handle);
+    __emval_free(handle);
   },
 
   _emval_new_array__deps: ['$Emval'],

src/library.js

@@ -3598,16 +3598,21 @@ addToLibrary({
 #endif
   },
 
+  // sentinel for invalid handles; it's intentionally a distinct object
+  // so that we could distinguish it from `undefined` as an actual stored value
+  $invalidHandleSentinel: {},
+
+  $handleAllocatorInit__deps: ['$invalidHandleSentinel'],
   $handleAllocatorInit: function() {
     Object.assign(HandleAllocator.prototype, /** @lends {HandleAllocator.prototype} */ {
       get(id) {
   #if ASSERTIONS
-        assert(this.allocated[id] !== undefined, `invalid handle: ${id}`);
+        assert(this.allocated[id] !== invalidHandleSentinel, `invalid handle: ${id}`);
   #endif
         return this.allocated[id];
       },
       has(id) {
-        return this.allocated[id] !== undefined;
+        return this.allocated[id] !== invalidHandleSentinel;
       },
       allocate(handle) {
         var id = this.freelist.pop() || this.allocated.length;
@@ -3616,22 +3621,20 @@ addToLibrary({
       },
       free(id) {
   #if ASSERTIONS
-        assert(this.allocated[id] !== undefined);
+        assert(this.allocated[id] !== invalidHandleSentinel);
   #endif
-        // Set the slot to `undefined` rather than using `delete` here since
-        // apparently arrays with holes in them can be less efficient.
-        this.allocated[id] = undefined;
+        this.allocated[id] = invalidHandleSentinel;
         this.freelist.push(id);
       }
     });
   },
 
   $HandleAllocator__postset: 'handleAllocatorInit()',
-  $HandleAllocator__deps: ['$handleAllocatorInit'],
+  $HandleAllocator__deps: ['$handleAllocatorInit', '$invalidHandleSentinel'],
   $HandleAllocator__docs: '/** @constructor */',
   $HandleAllocator: function() {
     // Reserve slot 0 so that 0 is always an invalid handle
-    this.allocated = [undefined];
+    this.allocated = [invalidHandleSentinel];
     this.freelist = [];
   },
 

src/library_sigs.js

@@ -339,16 +339,15 @@ sigs = {
   _emval_await__sig: 'pp',
   _emval_call__sig: 'ppipp',
   _emval_call_method__sig: 'dppppp',
-  _emval_decref__sig: 'vp',
   _emval_delete__sig: 'ipp',
   _emval_equals__sig: 'ipp',
+  _emval_free__sig: 'vp',
   _emval_get_global__sig: 'pp',
   _emval_get_method_caller__sig: 'pip',
   _emval_get_module_property__sig: 'pp',
   _emval_get_property__sig: 'ppp',
   _emval_greater_than__sig: 'ipp',
   _emval_in__sig: 'ipp',
-  _emval_incref__sig: 'vp',
   _emval_instanceof__sig: 'ipp',
   _emval_is_number__sig: 'ip',
   _emval_is_string__sig: 'ip',

system/include/emscripten/val.h

@@ -48,8 +48,7 @@ typedef struct _EM_METHOD_CALLER* EM_METHOD_CALLER;
 typedef double EM_GENERIC_WIRE_TYPE;
 typedef const void* EM_VAR_ARGS;
 
-void _emval_incref(EM_VAL value);
-void _emval_decref(EM_VAL value);
+void _emval_free(EM_VAL object);
 
 void _emval_run_destructors(EM_DESTRUCTORS handle);
 
@@ -287,6 +286,63 @@ struct MethodCaller {
   }
 };
 
+struct val_metadata {
+public:
+  val_metadata(EM_VAL handle)
+    : refcount(1)
+    , thread(pthread_self())
+    , handle(handle)
+  {}
+
+  // automatically deletes copy constructors and assignment operators as well
+  val_metadata(val_metadata&&) = delete;
+
+  val_metadata* inc_ref() {
+    ++refcount;
+    return this;
+  }
+
+  void dec_ref() {
+    if (--refcount == 0) {
+      delete this;
+    }
+  }
+
+  constexpr EM_VAL as_handle() const {
+#ifdef _REENTRANT
+    assert(pthread_equal(thread, pthread_self()) && "val accessed from wrong thread");
+#endif
+    return handle;
+  }
+
+  // Elimination-friendly overrides of operator new and delete.
+  // These intentionally do less work than the default implementations,
+  // which need to support custom handlers for out-of-memory conditions,
+  // while malloc/free pairs get inlined and can be and are eliminated
+  // by LLVM easily.
+  // Since most our values are temporary, we really want this optimisation.
+
+  void* operator new(size_t count) noexcept {
+    auto ptr = malloc(count * sizeof(val_metadata));
+    if (!ptr) abort();
+    return ptr;
+  }
+
+  void operator delete(void* ptr) noexcept {
+    free(ptr);
+  }
+
+private:
+  size_t refcount;
+  pthread_t thread;
+  EM_VAL handle;
+
+  // should be only accessible from dec_ref
+  ~val_metadata() {
+    _emval_free(handle);
+  }
+};
+
 } // end namespace internal
 
 #define EMSCRIPTEN_SYMBOL(name)                                         \
@@ -386,30 +442,16 @@ class val {
       : val(internal::_emval_new_cstring(v))
   {}
 
-  // Note: unlike other constructors, this doesn't use as_handle() because
-  // it just moves a value and doesn't need to go via incref/decref.
-  // This means it's safe to move values across threads - an error will
-  // only arise if you access or free it from the wrong thread later.
-  val(val&& v) : handle(v.handle), thread(v.thread) {
-    v.handle = 0;
-  }
+  val(val&& v) : data(v.data->inc_ref()) {}
 
-  val(const val& v) : val(v.as_handle()) {
-    internal::_emval_incref(handle);
-  }
+  val(const val& v) : val(std::move(v)) {}
 
   ~val() {
-    if (handle) {
-      internal::_emval_decref(as_handle());
-      handle = 0;
-    }
+    data->dec_ref();
   }
 
   EM_VAL as_handle() const {
-#ifdef _REENTRANT
-    assert(pthread_equal(thread, pthread_self()) && "val accessed from wrong thread");
-#endif
-    return handle;
+    return data->as_handle();
   }
 
   val& operator=(val&& v) & {
@@ -597,7 +639,7 @@ class val {
 private:
   // takes ownership, assumes handle already incref'd and lives on the same thread
   explicit val(EM_VAL handle)
-      : handle(handle), thread(pthread_self())
+      : data(new internal::val_metadata(handle))
   {}
 
   template<typename WrapperType>
@@ -621,8 +663,7 @@ class val {
     return v;
   }
 
-  pthread_t thread;
-  EM_VAL handle;
+  internal::val_metadata* data;
 
   friend struct internal::BindingType<val>;
 };
@@ -662,9 +703,7 @@ struct BindingType<T, typename std::enable_if<std::is_base_of<val, T>::value &&
                                               !std::is_const<T>::value>::type> {
   typedef EM_VAL WireType;
   static WireType toWireType(const val& v) {
-    EM_VAL handle = v.as_handle();
-    _emval_incref(handle);
-    return handle;
+    return v.as_handle();
   }
   static val fromWireType(WireType v) {
     return val::take_ownership(v);

test/test_other.py

@@ -14015,3 +14015,40 @@ def test_hello_world_argv(self):
 
   def test_arguments_global(self):
     self.emcc(test_file('hello_world_argv.c'), ['-sENVIRONMENT=web', '-sSTRICT', '--closure=1', '-O2'])
+
+  def test_emval_allocation_optimisations(self):
+    # Abort on any attempts to use dynamic allocation in runtime.
+    # This way we can check that LLVM optimised them all away.
+    # It's possible (but very unlikely) that in some future LLVM
+    # will regress in this optimisation, but it's more likely that
+    # we modify `val` in some way that makes those optimisations
+    # impossible - this is what the test is designed to help catch.
+    create_file('no_alloc.c', r'''
+      #include <stdlib.h>
+
+      void *malloc(size_t size) {
+        // Ensure all calls to malloc were compiled away when forbidden.
+        abort();
+      }
+
+      void free(void *ptr) {
+        // Same for free.
+        abort();
+      }
+    ''')
+    # Add this allocation implementation to compilation.
+    self.emcc_args += ['-xc', 'no_alloc.c']
+    create_file('test.cpp', r'''
+      #include <emscripten/val.h>
+
+      using emscripten::val;
+
+      int main() {
+        val::global("console").call<void>("log", val("Hello, world!"));
+      }
+    ''')
+    self.emcc_args += ['-xc++', '--bind']
+    # Test that even minimal optimisation level optimises allocations away
+    # for all those temporary emscripten::val values.
+    self.emcc_args += ['-O1']
+    self.do_runf('test.cpp', 'Hello, world!')