From 971a2424a0b1c7edadd53de990bfd2dbd16ef479 Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Wed, 19 Jul 2023 17:37:01 +0000 Subject: [PATCH 1/4] perf(jsTypeFactory): return the underlying JS function rather than wrapping it again if it's a wrapped JS function by us --- include/pyTypeFactory.hh | 2 +- src/jsTypeFactory.cc | 6 ++++++ src/pyTypeFactory.cc | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/pyTypeFactory.hh b/include/pyTypeFactory.hh index 90eac963..d8d94ab3 100644 --- a/include/pyTypeFactory.hh +++ b/include/pyTypeFactory.hh @@ -44,6 +44,6 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted *thisObj, JS::Rooted * @param args - Pointer to a PyTupleObject containing the arguments to the python function * @return PyObject* - The result of the JSFunction called with args coerced to JS types, coerced back to a PyObject type, or NULL if coercion wasn't possible */ -static PyObject *callJSFunc(PyObject *JSFuncAddress, PyObject *args); +PyObject *callJSFunc(PyObject *JSFuncAddress, PyObject *args); #endif \ No newline at end of file diff --git a/src/jsTypeFactory.cc b/src/jsTypeFactory.cc index 3105690f..d9535f94 100644 --- a/src/jsTypeFactory.cc +++ b/src/jsTypeFactory.cc @@ -117,6 +117,12 @@ JS::Value jsTypeFactory(JSContext *cx, PyObject *object) { } memoizePyTypeAndGCThing(new StrType(object), returnType); } + else if (PyCFunction_Check(object) && PyCFunction_GetFunction(object) == callJSFunc) { + // If it's a wrapped JS function by us, return the underlying JS function rather than wrapping it again + PyObject *jsCxThisFuncTuple = PyCFunction_GetSelf(object); + JS::RootedValue *jsFunc = (JS::RootedValue *)PyLong_AsVoidPtr(PyTuple_GetItem(jsCxThisFuncTuple, 2)); + returnType.set(*jsFunc); + } else if (PyFunction_Check(object) || PyCFunction_Check(object)) { // can't determine number of arguments for PyCFunctions, so just assume potentially unbounded uint16_t nargs = 0; diff --git a/src/pyTypeFactory.cc b/src/pyTypeFactory.cc index 1024f88d..3bffd4ca 100644 --- a/src/pyTypeFactory.cc +++ b/src/pyTypeFactory.cc @@ -167,7 +167,7 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted *thisObj, JS::Rooted return NULL; } -static PyObject *callJSFunc(PyObject *jsCxThisFuncTuple, PyObject *args) { +PyObject *callJSFunc(PyObject *jsCxThisFuncTuple, PyObject *args) { // TODO (Caleb Aikens) convert PyObject *args to JS::Rooted JSargs JSContext *cx = (JSContext *)PyLong_AsVoidPtr(PyTuple_GetItem(jsCxThisFuncTuple, 0)); JS::RootedObject *thisObj = (JS::RootedObject *)PyLong_AsVoidPtr(PyTuple_GetItem(jsCxThisFuncTuple, 1)); From 608b73ef7772a94d548ae978b6e3a4dacbe435ce Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Wed, 19 Jul 2023 17:38:32 +0000 Subject: [PATCH 2/4] perf(pyTypeFactory): return the underlying Python function rather than wrapping it again --- src/pyTypeFactory.cc | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/pyTypeFactory.cc b/src/pyTypeFactory.cc index 3bffd4ca..4185714f 100644 --- a/src/pyTypeFactory.cc +++ b/src/pyTypeFactory.cc @@ -32,6 +32,7 @@ #include "include/modules/pythonmonkey/pythonmonkey.hh" #include +#include #include #include @@ -123,9 +124,16 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted *thisObj, JS::Rooted return new ExceptionType(cx, obj); } case js::ESClass::Function: { - // FIXME (Tom Tang): `jsCxThisFuncTuple` and the tuple items are not going to be GCed - PyObject *jsCxThisFuncTuple = PyTuple_Pack(3, PyLong_FromVoidPtr(cx), PyLong_FromVoidPtr(thisObj), PyLong_FromVoidPtr(rval)); - PyObject *pyFunc = PyCFunction_New(&callJSFuncDef, jsCxThisFuncTuple); + PyObject *pyFunc; + if (JS_IsNativeFunction(obj, callPyFunc)) { // It's a wrapped python function by us + // Get the underlying python function from the 0th reserved slot + JS::Value pyFuncVal = js::GetFunctionNativeReserved(obj, 0); + pyFunc = (PyObject *)(pyFuncVal.toPrivate()); + } else { + // FIXME (Tom Tang): `jsCxThisFuncTuple` and the tuple items are not going to be GCed + PyObject *jsCxThisFuncTuple = PyTuple_Pack(3, PyLong_FromVoidPtr(cx), PyLong_FromVoidPtr(thisObj), PyLong_FromVoidPtr(rval)); + pyFunc = PyCFunction_New(&callJSFuncDef, jsCxThisFuncTuple); + } FuncType *f = new FuncType(pyFunc); memoizePyTypeAndGCThing(f, *rval); // TODO (Caleb Aikens) consider putting this in the FuncType constructor return f; From 88765ae468fd6a82a63cb510f1b4b0717486899d Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Wed, 19 Jul 2023 17:39:45 +0000 Subject: [PATCH 3/4] test(pyTypeFactory): write tests for JS calling Python functions defined in a closure --- tests/python/test_pythonmonkey_eval.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/python/test_pythonmonkey_eval.py b/tests/python/test_pythonmonkey_eval.py index 7d6be33e..a7386e83 100644 --- a/tests/python/test_pythonmonkey_eval.py +++ b/tests/python/test_pythonmonkey_eval.py @@ -235,6 +235,17 @@ def ident(x): # pm.collect() # TODO: to be fixed in BF-59 assert "YYZ" == js_fn_back() +def test_eval_functions_pyfunction_in_closure(): + # BF-58 https://github.com/Distributive-Network/PythonMonkey/pull/19 + def fn1(): + def fn0(n): + return n + 100 + return fn0 + assert 101.9 == fn1()(1.9) + assert 101.9 == pm.eval("(fn1) => { return fn1 }")(fn1())(1.9) + assert 101.9 == pm.eval("(fn1, x) => { return fn1()(x) }")(fn1, 1.9) + assert 101.9 == pm.eval("(fn1) => { return fn1() }")(fn1)(1.9) + def test_eval_functions_pyfunctions_ints(): caller = pm.eval("(func, param1, param2) => { return func(param1, param2) }") def add(a, b): From dd7126faa5cf42e468049ef94b2fdf20170074c3 Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Wed, 19 Jul 2023 17:49:49 +0000 Subject: [PATCH 4/4] test: write tests for unwrapping functions to their original language --- tests/python/test_pythonmonkey_eval.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/python/test_pythonmonkey_eval.py b/tests/python/test_pythonmonkey_eval.py index a7386e83..50601a29 100644 --- a/tests/python/test_pythonmonkey_eval.py +++ b/tests/python/test_pythonmonkey_eval.py @@ -246,6 +246,18 @@ def fn0(n): assert 101.9 == pm.eval("(fn1, x) => { return fn1()(x) }")(fn1, 1.9) assert 101.9 == pm.eval("(fn1) => { return fn1() }")(fn1)(1.9) +def test_unwrap_py_function(): + # https://github.com/Distributive-Network/PythonMonkey/issues/65 + def pyFunc(): + pass + unwrappedPyFunc = pm.eval("(wrappedPyFunc) => { return wrappedPyFunc }")(pyFunc) + assert unwrappedPyFunc is pyFunc + +def test_unwrap_js_function(): + # https://github.com/Distributive-Network/PythonMonkey/issues/65 + wrappedJSFunc = pm.eval("const JSFunc = () => { return 0 }\nJSFunc") + assert pm.eval("(unwrappedJSFunc) => { return unwrappedJSFunc === JSFunc }")(wrappedJSFunc) + def test_eval_functions_pyfunctions_ints(): caller = pm.eval("(func, param1, param2) => { return func(param1, param2) }") def add(a, b):