Use Uint32 counter in hiwire (#1016)

Also replaces JsERROR instances with NULL

src/core/hiwire.c

@@ -2,12 +2,6 @@
 
 #include "hiwire.h"
 
-JsRef
-hiwire_error()
-{
-  return Js_ERROR;
-}
-
 JsRef
 hiwire_undefined()
 {
@@ -15,7 +9,7 @@ hiwire_undefined()
 }
 
 JsRef
-hiwire_null()
+hiwire_jsnull()
 {
   return Js_NULL;
 }
@@ -39,16 +33,28 @@ hiwire_bool(bool boolean)
 }
 
 EM_JS(int, hiwire_init, (), {
-  let _hiwire = { objects : new Map(), counter : 1 };
+  let _hiwire = {
+    objects : new Map(),
+    // counter is used to allocate keys for the objects map.
+    // We use even integers to represent singleton constants which we won't
+    // reference count. We only want to allocate odd keys so we start at 1 and
+    // step by 2. We use a native uint32 for our counter, so counter
+    // automatically overflows back to 1 if it ever gets up to the max u32 =
+    // 2^{31} - 1. This ensures we can keep recycling keys even for very long
+    // sessions. (Also the native u32 is faster since javascript won't convert
+    // it to a float.)
+    // 0 == C NULL is an error code for compatibility with Python calling
+    // conventions.
+    counter : new Uint32Array([1])
+  };
   Module.hiwire = {};
-  Module.hiwire.ERROR = _hiwire_error();
   Module.hiwire.UNDEFINED = _hiwire_undefined();
-  Module.hiwire.NULL = _hiwire_null();
+  Module.hiwire.JSNULL = _hiwire_jsnull();
   Module.hiwire.TRUE = _hiwire_true();
   Module.hiwire.FALSE = _hiwire_false();
 
   _hiwire.objects.set(Module.hiwire.UNDEFINED, undefined);
-  _hiwire.objects.set(Module.hiwire.NULL, null);
+  _hiwire.objects.set(Module.hiwire.JSNULL, null);
   _hiwire.objects.set(Module.hiwire.TRUE, true);
   _hiwire.objects.set(Module.hiwire.FALSE, false);
 
@@ -58,12 +64,14 @@ EM_JS(int, hiwire_init, (), {
     // Probably not worth it for performance: it's harmless to ocassionally
     // duplicate. Maybe in test builds we could raise if jsval is a standard
     // value?
-    while (_hiwire.objects.has(_hiwire.counter)) {
+    while (_hiwire.objects.has(_hiwire.counter[0])) {
-      _hiwire.counter = (_hiwire.counter + 1) & 0x7fffffff;
+      // Increment by two here (and below) because even integers are reserved
+      // for singleton constants
+      _hiwire.counter[0] += 2;
     }
-    let idval = _hiwire.counter;
+    let idval = _hiwire.counter[0];
     _hiwire.objects.set(idval, jsval);
-    _hiwire.counter = (_hiwire.counter + 1) & 0x7fffffff;
+    _hiwire.counter[0] += 2;
     return idval;
   };
 
@@ -73,14 +81,20 @@ EM_JS(int, hiwire_init, (), {
       throw new Error("Argument to hiwire.get_value is undefined");
     }
     if (!_hiwire.objects.has(idval)) {
+      // clang-format off
       throw new Error(`Undefined id ${ idval }`);
+      // clang-format on
     }
     return _hiwire.objects.get(idval);
   };
 
   Module.hiwire.decref = function(idval)
   {
-    if (idval < 0) {
+    // clang-format off
+    if ((idval & 1) === 0) {
+      // least significant bit unset ==> idval is a singleton.
+      // We don't reference count singletons.
+      // clang-format on
       return;
     }
     _hiwire.objects.delete(idval);
@@ -89,7 +103,11 @@ EM_JS(int, hiwire_init, (), {
 });
 
 EM_JS(JsRef, hiwire_incref, (JsRef idval), {
-  if (idval < 0) {
+  // clang-format off
+  if ((idval & 1) === 0) {
+    // least significant bit unset ==> idval is a singleton.
+    // We don't reference count singletons.
+    // clang-format on
     return;
   }
   return Module.hiwire.new_value(Module.hiwire.get_value(idval));

src/core/hiwire.h

@@ -32,13 +32,13 @@ struct _JsRefStruct
 
 typedef struct _JsRefStruct* JsRef;
 
-// Define special ids for singleton constants. These must be negative to
+// Special JsRefs for singleton constants.
-// avoid being reused for other values.
+// (These must be even because the least significance bit is set to 0 for
-#define Js_ERROR ((JsRef)(-1))
+// singleton constants.)
-#define Js_UNDEFINED ((JsRef)(-2))
+#define Js_UNDEFINED ((JsRef)(2))
-#define Js_TRUE ((JsRef)(-3))
+#define Js_TRUE ((JsRef)(4))
-#define Js_FALSE ((JsRef)(-4))
+#define Js_FALSE ((JsRef)(6))
-#define Js_NULL ((JsRef)(-5))
+#define Js_NULL ((JsRef)(8))
 
 /**
  * Initialize the variables and functions required for hiwire.

src/core/jsimport.c

@@ -15,7 +15,7 @@ JsImport_GetAttr(PyObject* self, PyObject* attr)
     return NULL;
   }
   JsRef idval = hiwire_get_global(c);
-  if (idval == Js_ERROR) {
+  if (idval == NULL) {
     PyErr_Format(PyExc_AttributeError, "Unknown attribute '%s'", c);
     return NULL;
   }

src/core/jsproxy.c

@@ -70,7 +70,7 @@ JsProxy_GetAttr(PyObject* o, PyObject* attr_name)
   JsRef idresult = hiwire_get_member_string(self->js, key);
   Py_DECREF(str);
 
-  if (idresult == Js_ERROR) {
+  if (idresult == NULL) {
     PyErr_SetString(PyExc_AttributeError, key);
     return NULL;
   }
@@ -192,7 +192,7 @@ JsProxy_GetIter(PyObject* o)
 
   JsRef iditer = hiwire_get_iterator(self->js);
 
-  if (iditer == Js_ERROR) {
+  if (iditer == NULL) {
     PyErr_SetString(PyExc_TypeError, "Object is not iterable");
     return NULL;
   }
@@ -206,7 +206,7 @@ JsProxy_IterNext(PyObject* o)
   JsProxy* self = (JsProxy*)o;
 
   JsRef idresult = hiwire_next(self->js);
-  if (idresult == Js_ERROR) {
+  if (idresult == NULL) {
     return NULL;
   }
 
@@ -263,7 +263,7 @@ JsProxy_subscript(PyObject* o, PyObject* pyidx)
   JsRef ididx = python2js(pyidx);
   JsRef idresult = hiwire_get_member_obj(self->js, ididx);
   hiwire_decref(ididx);
-  if (idresult == Js_ERROR) {
+  if (idresult == NULL) {
     PyErr_SetObject(PyExc_KeyError, pyidx);
     return NULL;
   }

src/core/pyproxy.c

@@ -58,7 +58,7 @@ _pyproxy_set(PyObject* pyobj, JsRef idkey, JsRef idval)
 
   if (result) {
     pythonexc2js();
-    return Js_ERROR;
+    return NULL;
   }
   return idval;
 }
@@ -78,7 +78,7 @@ _pyproxy_deleteProperty(PyObject* pyobj, JsRef idkey)
 
   if (ret) {
     pythonexc2js();
-    return Js_ERROR;
+    return NULL;
   }
 
   return hiwire_undefined();
@@ -91,7 +91,7 @@ _pyproxy_ownKeys(PyObject* pyobj)
 
   if (pydir == NULL) {
     pythonexc2js();
-    return Js_ERROR;
+    return NULL;
   }
 
   JsRef iddir = hiwire_array();
@@ -128,7 +128,7 @@ _pyproxy_apply(PyObject* pyobj, JsRef idargs)
   if (pyresult == NULL) {
     Py_DECREF(pyargs);
     pythonexc2js();
-    return Js_ERROR;
+    return NULL;
   }
   JsRef idresult = python2js(pyresult);
   Py_DECREF(pyresult);

src/core/python2js.c

@@ -24,7 +24,7 @@ pythonexc2js()
   PyErr_Fetch(&type, &value, &traceback);
   PyErr_NormalizeException(&type, &value, &traceback);
 
-  JsRef excval = Js_ERROR;
+  JsRef excval = NULL;
   int exc;
 
   if (type == NULL || type == Py_None || value == NULL || value == Py_None) {
@@ -103,7 +103,7 @@ _python2js_float(PyObject* x)
 {
   double x_double = PyFloat_AsDouble(x);
   if (x_double == -1.0 && PyErr_Occurred()) {
-    return Js_ERROR;
+    return NULL;
   }
   return hiwire_double(x_double);
 }
@@ -117,11 +117,11 @@ _python2js_long(PyObject* x)
     if (overflow) {
       PyObject* py_float = PyNumber_Float(x);
       if (py_float == NULL) {
-        return Js_ERROR;
+        return NULL;
       }
       return _python2js_float(py_float);
     } else if (PyErr_Occurred()) {
-      return Js_ERROR;
+      return NULL;
     }
   }
   return hiwire_int(x_long);
@@ -142,7 +142,7 @@ _python2js_unicode(PyObject* x)
       return hiwire_string_ucs4(data, length);
     default:
       PyErr_SetString(PyExc_ValueError, "Unknown Unicode KIND");
-      return Js_ERROR;
+      return NULL;
   }
 }
 
@@ -152,7 +152,7 @@ _python2js_bytes(PyObject* x)
   char* x_buff;
   Py_ssize_t length;
   if (PyBytes_AsStringAndSize(x, &x_buff, &length)) {
-    return Js_ERROR;
+    return NULL;
   }
   return hiwire_bytes(x_buff, length);
 }
@@ -163,7 +163,7 @@ _python2js_sequence(PyObject* x, PyObject* map)
   JsRef jsarray = hiwire_array();
   if (_python2js_add_to_cache(map, x, jsarray)) {
     hiwire_decref(jsarray);
-    return Js_ERROR;
+    return NULL;
   }
   size_t length = PySequence_Size(x);
   for (size_t i = 0; i < length; ++i) {
@@ -178,11 +178,11 @@ _python2js_sequence(PyObject* x, PyObject* map)
       return pyproxy_new(x);
     }
     JsRef jsitem = _python2js_cache(pyitem, map);
-    if (jsitem == Js_ERROR) {
+    if (jsitem == NULL) {
       _python2js_remove_from_cache(map, x);
       Py_DECREF(pyitem);
       hiwire_decref(jsarray);
-      return Js_ERROR;
+      return NULL;
     }
     Py_DECREF(pyitem);
     hiwire_push_array(jsarray, jsitem);
@@ -190,7 +190,7 @@ _python2js_sequence(PyObject* x, PyObject* map)
   }
   if (_python2js_remove_from_cache(map, x)) {
     hiwire_decref(jsarray);
-    return Js_ERROR;
+    return NULL;
   }
   return jsarray;
 }
@@ -201,23 +201,23 @@ _python2js_dict(PyObject* x, PyObject* map)
   JsRef jsdict = hiwire_object();
   if (_python2js_add_to_cache(map, x, jsdict)) {
     hiwire_decref(jsdict);
-    return Js_ERROR;
+    return NULL;
   }
   PyObject *pykey, *pyval;
   Py_ssize_t pos = 0;
   while (PyDict_Next(x, &pos, &pykey, &pyval)) {
     JsRef jskey = _python2js_cache(pykey, map);
-    if (jskey == Js_ERROR) {
+    if (jskey == NULL) {
       _python2js_remove_from_cache(map, x);
       hiwire_decref(jsdict);
-      return Js_ERROR;
+      return NULL;
     }
     JsRef jsval = _python2js_cache(pyval, map);
-    if (jsval == Js_ERROR) {
+    if (jsval == NULL) {
       _python2js_remove_from_cache(map, x);
       hiwire_decref(jskey);
       hiwire_decref(jsdict);
-      return Js_ERROR;
+      return NULL;
     }
     hiwire_push_object_pair(jsdict, jskey, jsval);
     hiwire_decref(jskey);
@@ -225,7 +225,7 @@ _python2js_dict(PyObject* x, PyObject* map)
   }
   if (_python2js_remove_from_cache(map, x)) {
     hiwire_decref(jsdict);
-    return Js_ERROR;
+    return NULL;
   }
   return jsdict;
 }
@@ -258,7 +258,7 @@ _python2js(PyObject* x, PyObject* map)
   } else {
     JsRef ret = _python2js_buffer(x);
 
-    if (ret != Js_ERROR) {
+    if (ret != NULL) {
       return ret;
     }
     if (PySequence_Check(x)) {
@@ -269,7 +269,7 @@ _python2js(PyObject* x, PyObject* map)
     // same object on the Python side is always the same object on the
     // Javascript side.
     ret = pyproxy_use(x);
-    if (ret != Js_ERROR) {
+    if (ret != NULL) {
       return ret;
     }
 
@@ -322,7 +322,7 @@ _python2js_cache(PyObject* x, PyObject* map)
   JsRef result;
   if (val) {
     result = (JsRef)PyLong_AsLong(val);
-    if (result != Js_ERROR) {
+    if (result != NULL) {
       result = hiwire_incref(result);
     }
   } else {
@@ -339,7 +339,7 @@ python2js(PyObject* x)
   JsRef result = _python2js_cache(x, map);
   Py_DECREF(map);
 
-  if (result == Js_ERROR) {
+  if (result == NULL) {
     pythonexc2js();
   }
 

src/core/python2js_buffer.c

@@ -294,9 +294,9 @@ _python2js_buffer_recursive(Py_buffer* buff,
 
   for (i = 0; i < n; ++i) {
     jsitem = _python2js_buffer_recursive(buff, ptr, dim + 1, convert);
-    if (jsitem == Js_ERROR) {
+    if (jsitem == NULL) {
       hiwire_decref(jsarray);
-      return Js_ERROR;
+      return NULL;
     }
     hiwire_push_array(jsarray, jsitem);
     hiwire_decref(jsitem);
@@ -321,7 +321,7 @@ _python2js_buffer_to_typed_array(Py_buffer* buff)
       case '>':
       case '!':
         // This path can't handle byte-swapping
-        return Js_ERROR;
+        return NULL;
       case '=':
       case '<':
       case '@':
@@ -339,7 +339,7 @@ _python2js_buffer_to_typed_array(Py_buffer* buff)
     case 'B':
       return hiwire_uint8array((u8*)buff->buf, buff->len);
     case '?':
-      return Js_ERROR;
+      return NULL;
     case 'h':
       return hiwire_int16array((i16*)buff->buf, buff->len);
     case 'H':
@@ -354,13 +354,13 @@ _python2js_buffer_to_typed_array(Py_buffer* buff)
       return hiwire_uint32array((u32*)buff->buf, buff->len);
     case 'q':
     case 'Q':
-      return Js_ERROR;
+      return NULL;
     case 'f':
       return hiwire_float32array((f32*)buff->buf, buff->len);
     case 'd':
       return hiwire_float64array((f64*)buff->buf, buff->len);
     default:
-      return Js_ERROR;
+      return NULL;
   }
 }
 
@@ -408,9 +408,9 @@ _python2js_shareable_buffer_recursive(Py_buffer* buff,
   for (i = 0; i < n; ++i) {
     jsitem = _python2js_shareable_buffer_recursive(
       buff, shareable, idarr, ptr, dim + 1);
-    if (jsitem == Js_ERROR) {
+    if (jsitem == NULL) {
       hiwire_decref(jsarray);
-      return Js_ERROR;
+      return NULL;
     }
     hiwire_push_array(jsarray, jsitem);
     hiwire_decref(jsitem);
@@ -457,7 +457,7 @@ _python2js_buffer(PyObject* x)
   PyObject* memoryview = PyMemoryView_FromObject(x);
   if (memoryview == NULL) {
     PyErr_Clear();
-    return Js_ERROR;
+    return NULL;
   }
 
   Py_buffer* buff;
@@ -468,11 +468,11 @@ _python2js_buffer(PyObject* x)
 
   if (shareable != NOT_SHAREABLE) {
     JsRef idarr = _python2js_buffer_to_typed_array(buff);
-    if (idarr == Js_ERROR) {
+    if (idarr == NULL) {
       PyErr_SetString(
         PyExc_TypeError,
         "Internal error: Invalid type to convert to array buffer.");
-      return Js_ERROR;
+      return NULL;
     }
 
     result =
@@ -481,7 +481,7 @@ _python2js_buffer(PyObject* x)
     scalar_converter* convert = _python2js_buffer_get_converter(buff);
     if (convert == NULL) {
       Py_DECREF(memoryview);
-      return Js_ERROR;
+      return NULL;
     }
 
     result = _python2js_buffer_recursive(buff, buff->buf, 0, convert);

src/core/runpython.c

@@ -74,8 +74,8 @@ int
 runpython_init()
 {
   bool success = false;
-  JsRef pyodide_py_proxy = Js_ERROR;
+  JsRef pyodide_py_proxy = NULL;
-  JsRef globals_proxy = Js_ERROR;
+  JsRef globals_proxy = NULL;
 
   // borrowed
   PyObject* builtins = PyImport_AddModule("builtins");
@@ -101,7 +101,7 @@ runpython_init()
   QUIT_IF_NULL(pyodide_py);
 
   pyodide_py_proxy = python2js(pyodide_py);
-  if (pyodide_py_proxy == Js_ERROR) {
+  if (pyodide_py_proxy == NULL) {
     goto fail;
   }
   // Currently by default, python2js copies dicts into objects.
@@ -114,7 +114,7 @@ runpython_init()
   // modifications.
   Py_INCREF(globals); // pyproxy_new steals argument
   globals_proxy = pyproxy_new(globals);
-  if (globals_proxy == Js_ERROR) {
+  if (globals_proxy == NULL) {
     goto fail;
   }
   QUIT_IF_NZ(runpython_init_js(pyodide_py_proxy, globals_proxy));