From f43fc42f35f293c11333837acf2ff512930735ec Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Mon, 4 Jan 2021 14:07:40 -0800 Subject: [PATCH] Use Uint32 counter in hiwire (#1016) Also replaces JsERROR instances with NULL --- src/core/hiwire.c | 54 ++++++++++++++++++++++++------------- src/core/hiwire.h | 14 +++++----- src/core/jsimport.c | 2 +- src/core/jsproxy.c | 8 +++--- src/core/pyproxy.c | 8 +++--- src/core/python2js.c | 40 +++++++++++++-------------- src/core/python2js_buffer.c | 24 ++++++++--------- src/core/runpython.c | 8 +++--- 8 files changed, 88 insertions(+), 70 deletions(-) diff --git a/src/core/hiwire.c b/src/core/hiwire.c index 14e220238..8aebb1f9f 100644 --- a/src/core/hiwire.c +++ b/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)) { - _hiwire.counter = (_hiwire.counter + 1) & 0x7fffffff; + while (_hiwire.objects.has(_hiwire.counter[0])) { + // 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)) { - throw new Error(`Undefined id $ { 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)); diff --git a/src/core/hiwire.h b/src/core/hiwire.h index 5946dd4fa..7b988e122 100644 --- a/src/core/hiwire.h +++ b/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 -// avoid being reused for other values. -#define Js_ERROR ((JsRef)(-1)) -#define Js_UNDEFINED ((JsRef)(-2)) -#define Js_TRUE ((JsRef)(-3)) -#define Js_FALSE ((JsRef)(-4)) -#define Js_NULL ((JsRef)(-5)) +// Special JsRefs for singleton constants. +// (These must be even because the least significance bit is set to 0 for +// singleton constants.) +#define Js_UNDEFINED ((JsRef)(2)) +#define Js_TRUE ((JsRef)(4)) +#define Js_FALSE ((JsRef)(6)) +#define Js_NULL ((JsRef)(8)) /** * Initialize the variables and functions required for hiwire. diff --git a/src/core/jsimport.c b/src/core/jsimport.c index a000230a2..b1dee49a0 100644 --- a/src/core/jsimport.c +++ b/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; } diff --git a/src/core/jsproxy.c b/src/core/jsproxy.c index 468efddac..c6e9ba4b2 100644 --- a/src/core/jsproxy.c +++ b/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; } diff --git a/src/core/pyproxy.c b/src/core/pyproxy.c index 478929519..a4dda3c91 100644 --- a/src/core/pyproxy.c +++ b/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); diff --git a/src/core/python2js.c b/src/core/python2js.c index 4e61f3f43..02a930729 100644 --- a/src/core/python2js.c +++ b/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(); } diff --git a/src/core/python2js_buffer.c b/src/core/python2js_buffer.c index 3f187d612..487e0f55c 100644 --- a/src/core/python2js_buffer.c +++ b/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); diff --git a/src/core/runpython.c b/src/core/runpython.c index f11fc018c..46bf000d2 100644 --- a/src/core/runpython.c +++ b/src/core/runpython.c @@ -74,8 +74,8 @@ int runpython_init() { bool success = false; - JsRef pyodide_py_proxy = Js_ERROR; - JsRef globals_proxy = Js_ERROR; + JsRef pyodide_py_proxy = NULL; + 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));