From 46eee26ff8ffd8dd320e5582cc40d0f9af8ac80e Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 1 Jul 2021 10:47:44 -0700 Subject: [PATCH] Various improvements in core (#1673) --- cpython/Makefile | 10 +++++- packages/micropip/test_micropip.py | 2 +- packages/numpy/test_numpy.py | 12 ++++---- src/core/hiwire.c | 46 ++++++++++++++-------------- src/core/hiwire.h | 8 ++--- src/core/js2python.c | 25 +++++++++++++-- src/core/jsmemops.h | 49 ++++++++++++++++++++++++++++++ src/core/pyproxy.c | 18 +++++------ src/core/pyproxy.js | 46 ++++++++++++++-------------- src/core/python2js.c | 27 ++++++++-------- src/core/python2js_buffer.c | 1 + src/core/python2js_buffer.js | 12 ++++---- 12 files changed, 166 insertions(+), 90 deletions(-) create mode 100644 src/core/jsmemops.h diff --git a/cpython/Makefile b/cpython/Makefile index 712a4ab41..b4eb6f9e2 100644 --- a/cpython/Makefile +++ b/cpython/Makefile @@ -123,6 +123,13 @@ $(INSTALL)/lib/libffi.a : cp $(FFIBUILD)/target/lib/libffi.a $(INSTALL)/lib/ $(BUILD)/Makefile: $(BUILD)/.patched $(ZLIBBUILD)/.configured $(INSTALL)/lib/libsqlite3.a $(INSTALL)/lib/libbz2.a + # --enable-big-digits=30 : + # Python integers have "digits" of size 15 by default on systems with 32 + # bit pointers and size 30 on systems with 16 bit pointers. Python uses + # "digits" of size 15 by default on systems with 32 bit pointers and size + # 30 on systems with 16 bit pointers. WASM has 32 bit pointers so Python + # will default to the size 15 digits but WASM has native 64 bit arithmetic + # so it is more efficient to use 30 bit digits. cp config.site $(BUILD)/ ( \ cd $(BUILD); \ @@ -133,10 +140,11 @@ $(BUILD)/Makefile: $(BUILD)/.patched $(ZLIBBUILD)/.configured $(INSTALL)/lib/lib --without-pymalloc \ --disable-shared \ --disable-ipv6 \ + --enable-big-digits=30 \ --enable-optimizations \ --host=wasm32-unknown-emscripten\ --build=$(shell $(BUILD)/config.guess) \ - --prefix=$(INSTALL) ; \ + --prefix=$(INSTALL) \ ) diff --git a/packages/micropip/test_micropip.py b/packages/micropip/test_micropip.py index e81b54456..9d88b005c 100644 --- a/packages/micropip/test_micropip.py +++ b/packages/micropip/test_micropip.py @@ -14,7 +14,7 @@ def selenium_standalone_micropip(selenium_standalone): selenium_standalone.run_js( """ await pyodide.loadPackage("micropip"); - await pyodide.runPython("import micropip"); + pyodide.runPython("import micropip"); """ ) yield selenium_standalone diff --git a/packages/numpy/test_numpy.py b/packages/numpy/test_numpy.py index 3a184bb8b..394466d1f 100644 --- a/packages/numpy/test_numpy.py +++ b/packages/numpy/test_numpy.py @@ -209,7 +209,7 @@ def test_get_buffer(selenium): selenium.run_js( """ await pyodide.loadPackage(['numpy']); - await pyodide.runPython(` + pyodide.runPython(` import numpy as np x = np.arange(24) z1 = x.reshape([8,3]) @@ -218,7 +218,7 @@ def test_get_buffer(selenium): z4 = z1[-1::-1,-1::-1] `); for(let x of ["z1", "z2", "z3", "z4"]){ - let z = pyodide.pyimport(x).getBuffer("u32"); + let z = pyodide.globals.get(x).getBuffer("u32"); for(let idx1 = 0; idx1 < 8; idx1++) { for(let idx2 = 0; idx2 < 3; idx2++){ let v1 = z.data[z.offset + z.strides[0] * idx1 + z.strides[1] * idx2]; @@ -260,11 +260,11 @@ def test_get_buffer_roundtrip(selenium, arg): selenium.run_js( f""" await pyodide.loadPackage(['numpy']); - await pyodide.runPython(` + pyodide.runPython(` import numpy as np x = {arg} `); - window.x_js_buf = pyodide.pyimport("x").getBuffer(); + window.x_js_buf = pyodide.globals.get("x").getBuffer(); x_js_buf.length = x_js_buf.data.length; """ ) @@ -303,7 +303,7 @@ def test_get_buffer_big_endian(selenium): selenium.run_js( """ await pyodide.loadPackage(['numpy']); - window.a = await pyodide.runPython(` + window.a = pyodide.runPython(` import numpy as np np.arange(24, dtype="int16").byteswap().newbyteorder() `); @@ -331,7 +331,7 @@ def test_get_buffer_error_messages(selenium): selenium.run_js( """ await pyodide.loadPackage(['numpy']); - await pyodide.runPython(` + pyodide.runPython(` import numpy as np x = np.ones(2, dtype=np.float16) `); diff --git a/src/core/hiwire.c b/src/core/hiwire.c index 7808fcd25..ba638b876 100644 --- a/src/core/hiwire.c +++ b/src/core/hiwire.c @@ -5,6 +5,7 @@ #include #include "hiwire.h" +#include "jsmemops.h" #define ERROR_REF (0) #define ERROR_NUM (-1) @@ -39,10 +40,10 @@ EM_JS_NUM(int, hiwire_init, (), { counter : new Uint32Array([1]) }; Module.hiwire = {}; - Module.hiwire.UNDEFINED = HEAP8[_Js_undefined]; - Module.hiwire.JSNULL = HEAP8[_Js_null]; - Module.hiwire.TRUE = HEAP8[_Js_true]; - Module.hiwire.FALSE = HEAP8[_Js_false]; + Module.hiwire.UNDEFINED = DEREF_U8(_Js_undefined, 0); + Module.hiwire.JSNULL = DEREF_U8(_Js_null, 0); + Module.hiwire.TRUE = DEREF_U8(_Js_true, 0); + Module.hiwire.FALSE = DEREF_U8(_Js_false, 0); _hiwire.objects.set(Module.hiwire.UNDEFINED, undefined); _hiwire.objects.set(Module.hiwire.JSNULL, null); @@ -230,21 +231,21 @@ EM_JS_REF(JsRef, hiwire_int, (int val), { return Module.hiwire.new_value(val); }); -EM_JS_REF(JsRef, hiwire_int_from_hex, (const char* s), { - let result; - // clang-format off - // Check if number starts with a minus sign - if (HEAP8[s] === 45) { - // clang-format on - result = -Module.BigInt(UTF8ToString(s + 1)); - } else { - result = Module.BigInt(UTF8ToString(s)); +// clang-format off +EM_JS_REF(JsRef, +hiwire_int_from_digits, (const unsigned int* digits, size_t ndigits), { + let result = BigInt(0); + for (let i = 0; i < ndigits; i++) { + result += BigInt(DEREF_U32(digits, i)) << BigInt(32 * i); } + result += BigInt(DEREF_U32(digits, ndigits - 1) & 0x80000000) + << BigInt(1 + 32 * (ndigits - 1)); if (-Number.MAX_SAFE_INTEGER < result && result < Number.MAX_SAFE_INTEGER) { result = Number(result); } return Module.hiwire.new_value(result); -}); +}) +// clang-format on EM_JS_REF(JsRef, hiwire_double, (double val), { return Module.hiwire.new_value(val); @@ -252,27 +253,24 @@ EM_JS_REF(JsRef, hiwire_double, (double val), { EM_JS_REF(JsRef, hiwire_string_ucs4, (const char* ptr, int len), { let jsstr = ""; - let idx = ptr / 4; for (let i = 0; i < len; ++i) { - jsstr += String.fromCodePoint(Module.HEAPU32[idx + i]); + jsstr += String.fromCodePoint(DEREF_U32(ptr, i)); } return Module.hiwire.new_value(jsstr); }); EM_JS_REF(JsRef, hiwire_string_ucs2, (const char* ptr, int len), { let jsstr = ""; - let idx = ptr / 2; for (let i = 0; i < len; ++i) { - jsstr += String.fromCharCode(Module.HEAPU16[idx + i]); + jsstr += String.fromCharCode(DEREF_U16(ptr, i)); } return Module.hiwire.new_value(jsstr); }); EM_JS_REF(JsRef, hiwire_string_ucs1, (const char* ptr, int len), { let jsstr = ""; - let idx = ptr; for (let i = 0; i < len; ++i) { - jsstr += String.fromCharCode(Module.HEAPU8[idx + i]); + jsstr += String.fromCharCode(DEREF_U8(ptr, i)); } return Module.hiwire.new_value(jsstr); }); @@ -664,7 +662,7 @@ EM_JS_NUM(int, hiwire_next, (JsRef idobj, JsRef* result_ptr), { let { done, value } = jsobj.next(); // clang-format on let result_id = Module.hiwire.new_value(value); - setValue(result_ptr, result_id, "i32"); + DEREF_U32(result_ptr, 0) = result_id; return done; }); @@ -739,9 +737,9 @@ hiwire_get_buffer_datatype, let jsobj = Module.hiwire.get_value(idobj); let [format_utf8, size, checked] = Module.get_buffer_datatype(jsobj); // Store results into arguments - setValue(format_ptr, format_utf8, "i8*"); - setValue(size_ptr, size, "i32"); - setValue(checked_ptr, size, "i8"); + DEREF_U32(format_ptr, 0) = format_utf8; + DEREF_U32(size_ptr, 0) = size; + DEREF_U8(checked_ptr, 0) = checked; }); // clang-format on diff --git a/src/core/hiwire.h b/src/core/hiwire.h index b8b041648..945e68018 100644 --- a/src/core/hiwire.h +++ b/src/core/hiwire.h @@ -60,12 +60,12 @@ int hiwire_init(); /** - * Convert a string of hexadecimal digits to a Number or BigInt depending on - * whether it is less than MAX_SAFE_INTEGER or not. The string is assumed to - * begin with an optional sign followed by 0x followed by one or more digits. + * Convert an array of int32s to a Number or BigInt depending on whether it is + * less than MAX_SAFE_INTEGER or not. The representation is assumed to be signed + * and little endian. */ JsRef -hiwire_int_from_hex(const char* s); +hiwire_int_from_digits(const unsigned int* bytes, size_t nbytes); /** * Increase the reference count on an object. diff --git a/src/core/js2python.c b/src/core/js2python.c index 310c651fd..7eb8bfe2e 100644 --- a/src/core/js2python.c +++ b/src/core/js2python.c @@ -6,6 +6,7 @@ #include +#include "jsmemops.h" #include "jsproxy.h" #include "pyproxy.h" @@ -90,9 +91,27 @@ EM_JS_NUM(errcode, js2python_init, (), { Module.__js2python_bigint = function(value) { - let ptr = stringToNewUTF8(value.toString(16)); - let result = _PyLong_FromString(ptr, 0, 16); - _free(ptr); + let value_orig = value; + let length = 0; + if (value < 0) { + value = -value; + } + while (value) { + length++; + value >>= BigInt(32); + } + let stackTop = stackSave(); + let ptr = stackAlloc(length * 4); + value = value_orig; + for (let i = 0; i < length; i++) { + DEREF_U32(ptr, i) = Number(value & BigInt(0xffffffff)); + value >>= BigInt(32); + } + let result = __PyLong_FromByteArray(ptr, + length * 4 /* length in bytes */, + true /* little endian */, + true /* signed? */); + stackRestore(stackTop); return result; }; diff --git a/src/core/jsmemops.h b/src/core/jsmemops.h new file mode 100644 index 000000000..9bb97925f --- /dev/null +++ b/src/core/jsmemops.h @@ -0,0 +1,49 @@ +// Macros to access memory from Javascript + +#define DEREF_U8(addr, offset) HEAPU8[addr + offset] +#define DEREF_I8(addr, offset) HEAP8[addr + offset] + +#define DEREF_U16(addr, offset) HEAPU16[(addr >> 1) + offset] +#define DEREF_I16(addr, offset) HEAP16[(addr >> 1) + offset] + +#define DEREF_U32(addr, offset) HEAPU32[(addr >> 2) + offset] +#define DEREF_I32(addr, offset) HEAP32[(addr >> 2) + offset] + +#define DEREF_F32(addr, offset) HEAPF32[(addr >> 2) + offset] +#define DEREF_F64(addr, offset) HEAPF64[(addr >> 3) + offset] + +#if WASM_BIGINT +// We have HEAPU64 / HEAPI64 in this case. +#define DEREF_U64(addr, offset) HEAPU64[(addr >> 3) + offset] +#define DEREF_I64(addr, offset) HEAP64[(addr >> 3) + offset] + +#define LOAD_U64(addr, offset) DEREF_U64(addr, offset) +#define LOAD_I64(addr, offset) DEREF_I64(addr, offset) + +#define STORE_U64(addr, offset, val) (DEREF_U64(addr, offset) = val) +#define STORE_I64(addr, offset, val) (DEREF_U64(addr, offset) = val) + +#else + +// No BigUint64Array, have to manually split / join lower and upper byte +// +#define BIGINT_LOWER(x) (Number((x)&BigInt(0xffffffff)) | 0) +#define BIGINT_UPPER(x) (Number((x) >> BigInt(32)) | 0) +#define UBIGINT_FROM_PAIR(lower, upper) \ + (BigInt(lower) | (BigInt(upper) << BigInt(32))) + +#define IBIGINT_FROM_PAIR(lower, upper) \ + (BigInt(lower) | (BigInt(upper - 2 * (upper & 0x80000000)) << BigInt(32))) + +#define LOAD_U64(addr, offset) \ + UBIGINT_FROM_PAIR(DEREF_U32(addr, offset * 2), \ + DEREF_U32(addr, offset * 2 + 1)) +#define LOAD_I64(addr, offset) \ + IBIGINT_FROM_PAIR(DEREF_U32(addr, offset * 2), \ + DEREF_U32(addr, offset * 2 + 1)) + +#define STORE_U64(addr, offset, val) \ + ((DEREF_U32(addr, offset * 2) = BIGINT_LOWER(val)), \ + (DEREF_U32(addr, offset * 2 + 1) = BIGINT_UPPER(val))) +#define STORE_I64 STORE_U64 +#endif diff --git a/src/core/pyproxy.c b/src/core/pyproxy.c index e501e56e8..3df1312d6 100644 --- a/src/core/pyproxy.c +++ b/src/core/pyproxy.c @@ -6,6 +6,7 @@ #include "docstring.h" #include "hiwire.h" #include "js2python.h" +#include "jsmemops.h" // for pyproxy.js #include "jsproxy.h" #include "python2js.h" @@ -737,6 +738,8 @@ typedef struct int f_contiguous; } buffer_struct; +size_t buffer_struct_size = sizeof(buffer_struct); + /** * This is the C part of the getBuffer method. * @@ -757,8 +760,8 @@ typedef struct * We also put the various other metadata about the buffer that we want to share * into buffer_struct. */ -buffer_struct* -_pyproxy_get_buffer(PyObject* ptrobj) +int +_pyproxy_get_buffer(buffer_struct* target, PyObject* ptrobj) { Py_buffer view; // PyBUF_RECORDS_RO requires that suboffsets be NULL but otherwise is the most @@ -766,7 +769,7 @@ _pyproxy_get_buffer(PyObject* ptrobj) if (PyObject_GetBuffer(ptrobj, &view, PyBUF_RECORDS_RO) == -1) { // Buffer cannot be represented without suboffsets. The bf_getbuffer method // should have set a PyExc_BufferError saying something to this effect. - return NULL; + return -1; } bool success = false; @@ -830,16 +833,13 @@ finally: // Py_Buffer.release(). result.view = (Py_buffer*)PyMem_Malloc(sizeof(Py_buffer)); *result.view = view; - // The result_heap memory will be freed by getBuffer - buffer_struct* result_heap = - (buffer_struct*)PyMem_Malloc(sizeof(buffer_struct)); - *result_heap = result; - return result_heap; + *target = result; + return 0; } else { hiwire_CLEAR(result.shape); hiwire_CLEAR(result.strides); PyBuffer_Release(&view); - return NULL; + return -1; } } diff --git a/src/core/pyproxy.js b/src/core/pyproxy.js index 401b67681..fcf221d76 100644 --- a/src/core/pyproxy.js +++ b/src/core/pyproxy.js @@ -1080,41 +1080,39 @@ class PyProxyBufferMethods { throw new Error(`Unknown type ${type}`); } } + let HEAPU32 = Module.HEAPU32; + let orig_stack_ptr = Module.stackSave(); + let buffer_struct_ptr = Module.stackAlloc( + DEREF_U32(Module._buffer_struct_size, 0) + ); let this_ptr = _getPtr(this); - let buffer_struct_ptr; + let errcode; try { - buffer_struct_ptr = Module.__pyproxy_get_buffer(this_ptr); + errcode = Module.__pyproxy_get_buffer(buffer_struct_ptr, this_ptr); } catch (e) { Module.fatal_error(e); } - if (buffer_struct_ptr === 0) { + if (errcode === -1) { Module._pythonexc2js(); } - let HEAP32 = Module.HEAP32; - // This has to match the order of the fields in buffer_struct - let cur_ptr = buffer_struct_ptr / 4; + // This has to match the fields in buffer_struct + let startByteOffset = DEREF_U32(buffer_struct_ptr, 0); + let minByteOffset = DEREF_U32(buffer_struct_ptr, 1); + let maxByteOffset = DEREF_U32(buffer_struct_ptr, 2); - let startByteOffset = HEAP32[cur_ptr++]; - let minByteOffset = HEAP32[cur_ptr++]; - let maxByteOffset = HEAP32[cur_ptr++]; + let readonly = !!DEREF_U32(buffer_struct_ptr, 3); + let format_ptr = DEREF_U32(buffer_struct_ptr, 4); + let itemsize = DEREF_U32(buffer_struct_ptr, 5); + let shape = Module.hiwire.pop_value(DEREF_U32(buffer_struct_ptr, 6)); + let strides = Module.hiwire.pop_value(DEREF_U32(buffer_struct_ptr, 7)); - let readonly = !!HEAP32[cur_ptr++]; - let format_ptr = HEAP32[cur_ptr++]; - let itemsize = HEAP32[cur_ptr++]; - let shape = Module.hiwire.pop_value(HEAP32[cur_ptr++]); - let strides = Module.hiwire.pop_value(HEAP32[cur_ptr++]); - - let view_ptr = HEAP32[cur_ptr++]; - let c_contiguous = !!HEAP32[cur_ptr++]; - let f_contiguous = !!HEAP32[cur_ptr++]; + let view_ptr = DEREF_U32(buffer_struct_ptr, 8); + let c_contiguous = !!DEREF_U32(buffer_struct_ptr, 9); + let f_contiguous = !!DEREF_U32(buffer_struct_ptr, 10); let format = Module.UTF8ToString(format_ptr); - try { - Module._PyMem_Free(buffer_struct_ptr); - } catch (e) { - Module.fatal_error(e); - } + Module.stackRestore(orig_stack_ptr); let success = false; try { @@ -1153,7 +1151,7 @@ class PyProxyBufferMethods { if (numBytes === 0) { data = new ArrayType(); } else { - data = new ArrayType(HEAP32.buffer, minByteOffset, numEntries); + data = new ArrayType(HEAPU32.buffer, minByteOffset, numEntries); } for (let i of strides.keys()) { strides[i] /= alignment; diff --git a/src/core/python2js.c b/src/core/python2js.c index a2739f414..58440d1b7 100644 --- a/src/core/python2js.c +++ b/src/core/python2js.c @@ -40,25 +40,28 @@ _python2js_float(PyObject* x) return hiwire_double(x_double); } +#if PYLONG_BITS_IN_DIGIT == 15 +#error "Expected PYLONG_BITS_IN_DIGIT == 30" +#endif + static JsRef _python2js_long(PyObject* x) { int overflow; long x_long = PyLong_AsLongAndOverflow(x, &overflow); if (x_long == -1) { - if (overflow) { - // Backup approach for large integers: convert via hex string. - // - // Unfortunately Javascript doesn't offer a good way to convert a numbers - // to / from Uint8Arrays. - PyObject* hex_py = PyNumber_ToBase(x, 16); - FAIL_IF_NULL(hex_py); - const char* hex_str = PyUnicode_AsUTF8(hex_py); - JsRef result = hiwire_int_from_hex(hex_str); - Py_DECREF(hex_py); - return result; + if (!overflow) { + FAIL_IF_ERR_OCCURRED(); + } else { + size_t ndigits = Py_ABS(Py_SIZE(x)); + unsigned int digits[ndigits]; + FAIL_IF_MINUS_ONE(_PyLong_AsByteArray((PyLongObject*)x, + (unsigned char*)digits, + 4 * ndigits, + true /* little endian */, + true /* signed */)); + return hiwire_int_from_digits(digits, ndigits); } - FAIL_IF_ERR_OCCURRED(); } return hiwire_int(x_long); finally: diff --git a/src/core/python2js_buffer.c b/src/core/python2js_buffer.c index 4941e36e6..e7813ac4f 100644 --- a/src/core/python2js_buffer.c +++ b/src/core/python2js_buffer.c @@ -9,6 +9,7 @@ #include #include "hiwire.h" +#include "jsmemops.h" // This file handles the conversion of Python buffer objects (which loosely // represent Numpy arrays) to Javascript. diff --git a/src/core/python2js_buffer.js b/src/core/python2js_buffer.js index 0e84d35e2..b5f7f1838 100644 --- a/src/core/python2js_buffer.js +++ b/src/core/python2js_buffer.js @@ -152,7 +152,7 @@ JS_FILE(python2js_buffer_init, () => { for (let i = 0; i < n; ++i) { let curptr = ptr + i * stride; if (suboffset >= 0) { - curptr = HEAP32[curptr / 4] + suboffset; + curptr = DEREF_U32(curptr, 0) + suboffset; } buffer.set(HEAP8.subarray(curptr, curptr + itemsize), i * itemsize); } @@ -181,12 +181,12 @@ JS_FILE(python2js_buffer_init, () => { */ Module._python2js_buffer_recursive = function (ptr, curdim, bufferData) { "use strict"; - // When indexing HEAP32 we need to divide the pointer by 4 - let n = HEAP32[bufferData.shape / 4 + curdim]; - let stride = HEAP32[bufferData.strides / 4 + curdim]; + // Stride and suboffset are signed, n is unsigned. + let n = DEREF_U32(bufferData.shape, curdim); + let stride = DEREF_I32(bufferData.strides, curdim); let suboffset = -1; if (bufferData.suboffsets !== 0) { - suboffset = HEAP32[bufferData.suboffsets / 4 + curdim]; + suboffset = DEREF_I32(bufferData.suboffsets, curdim); } if (curdim === bufferData.ndim - 1) { // Last dimension, use appropriate 1d converter @@ -211,7 +211,7 @@ JS_FILE(python2js_buffer_init, () => { // https://docs.python.org/3/c-api/buffer.html#pil-style-shape-strides-and-suboffsets let curPtr = ptr + i * stride; if (suboffset >= 0) { - curptr = HEAP32[curptr / 4] + suboffset; + curptr = DEREF_U32(curptr, 0) + suboffset; } result.push( Module._python2js_buffer_recursive(curPtr, curdim + 1, bufferData)