PERF Only render extra destroyed information when debug mode is on

docs/project/changelog.md

@@ -15,6 +15,11 @@ myst:
 
 ## Unreleased
 
+- {{ Enhancement }} For performance reasons, don't render extra information in
+  PyProxy destroyed message by default. By using `pyodide.setDebug(true)`, you
+  can opt into worse performance and better error messages.
+  {pr}`4027`
+
 - {{ Fix }} Fixed adding getters/setters to a `PyProxy` with
   `Object.defineProperty` and improved compliance with JavaScript rules around
   Proxy traps.

src/core/pyproxy.ts

@@ -380,6 +380,35 @@ function pyproxy_decref_cache(cache: PyProxyCache) {
   }
 }
 
+function generateDestroyedMessage(
+  proxy: PyProxy,
+  destroyed_msg: string,
+): string {
+  destroyed_msg = destroyed_msg || "Object has already been destroyed";
+  if (API.debug_ffi) {
+    let proxy_type = proxy.type;
+    let proxy_repr;
+    try {
+      proxy_repr = proxy.toString();
+    } catch (e) {
+      if ((e as any).pyodide_fatal_error) {
+        throw e;
+      }
+    }
+    destroyed_msg += "\n" + `The object was of type "${proxy_type}" and `;
+    if (proxy_repr) {
+      destroyed_msg += `had repr "${proxy_repr}"`;
+    } else {
+      destroyed_msg += "an error was raised when trying to generate its repr";
+    }
+  } else {
+    destroyed_msg +=
+      "\n" +
+      "For more information about the cause of this error, use `pyodide.setDebug(true)`";
+  }
+  return destroyed_msg;
+}
+
 Module.pyproxy_destroy = function (
   proxy: PyProxy,
   destroyed_msg: string,
@@ -393,29 +422,12 @@ Module.pyproxy_destroy = function (
   }
   let ptrobj = _getPtr(proxy);
   Module.finalizationRegistry.unregister(proxy.$$);
-  destroyed_msg = destroyed_msg || "Object has already been destroyed";
-  let proxy_type = proxy.type;
-  let proxy_repr;
-  try {
-    proxy_repr = proxy.toString();
-  } catch (e) {
-    if ((e as any).pyodide_fatal_error) {
-      throw e;
-    }
-  }
+  proxy.$$.destroyed_msg = generateDestroyedMessage(proxy, destroyed_msg);
+  pyproxy_decref_cache(proxy.$$.cache);
   // Maybe the destructor will call JavaScript code that will somehow try
   // to use this proxy. Mark it deleted before decrementing reference count
   // just in case!
   proxy.$$.ptr = 0;
-  Module.finalizationRegistry.unregister(proxy.$$);
-  destroyed_msg += "\n" + `The object was of type "${proxy_type}" and `;
-  if (proxy_repr) {
-    destroyed_msg += `had repr "${proxy_repr}"`;
-  } else {
-    destroyed_msg += "an error was raised when trying to generate its repr";
-  }
-  proxy.$$.destroyed_msg = destroyed_msg;
-  pyproxy_decref_cache(proxy.$$.cache);
   try {
     Py_ENTER();
     Module._Py_DecRef(ptrobj);

src/js/api.ts

@@ -626,6 +626,18 @@ export class PyodideAPI {
     Object.defineProperty(this, "PythonError", { value: PythonError });
     return PythonError;
   }
+
+  /**
+   * Turn on or off debug mode. In debug mode, some error messages are improved
+   * at a performance cost.
+   * @param debug If true, turn debug mode on. If false, turn debug mode off.
+   * @returns The old value of the debug flag.
+   */
+  static setDebug(debug: boolean): boolean {
+    const orig = !!API.debug_ffi;
+    API.debug_ffi = debug;
+    return orig;
+  }
 }
 
 /** @hidetype */

src/tests/test_jsproxy.py

@@ -348,6 +348,7 @@ def test_call_pyproxy_destroy_args(selenium):
     selenium.run_js(
         r"""
         let y;
+        pyodide.setDebug(true);
         self.f = function(x){ y = x; }
         pyodide.runPython(`
             from js import f
@@ -361,6 +362,23 @@ def test_call_pyproxy_destroy_args(selenium):
         """
     )
 
+    selenium.run_js(
+        r"""
+        let y;
+        pyodide.setDebug(false);
+        self.f = function(x){ y = x; }
+        pyodide.runPython(`
+            from js import f
+            f({})
+            f([])
+        `);
+        assertThrows(() => y.length, "Error",
+            "This borrowed proxy was automatically destroyed at the end of a function call.*\n" +
+            'For more information about the cause of this error, use `pyodide.setDebug.true.`'
+        );
+        """
+    )
+
     selenium.run_js(
         """
         let y;

src/tests/test_pyodide.py

@@ -744,7 +744,7 @@ def test_return_destroyed_value(selenium):
     f = run_js("(function(x){ return x; })")
     p = create_proxy([])
     p.destroy()
-    with pytest.raises(JsException, match='The object was of type "list" and had repr'):
+    with pytest.raises(JsException, match="Object has already been destroyed"):
         f(p)
 
 

src/tests/test_pyproxy.py

@@ -856,50 +856,55 @@ def test_pyproxy_implicit_copy(selenium):
 def test_errors(selenium):
     selenium.run_js(
         r"""
-        let t = pyodide.runPython(`
-            from pyodide.ffi import to_js
-            def te(self, *args, **kwargs):
-                raise Exception(repr(args))
-            class Temp:
-                __getattr__ = te
-                __setattr__ = te
-                __delattr__ = te
-                __dir__ = te
-                __call__ = te
-                __getitem__ = te
-                __setitem__ = te
-                __delitem__ = te
-                __iter__ = te
-                __len__ = te
-                __contains__ = te
-                __await__ = te
-                __repr__ = te
-            to_js(Temp())
-            Temp()
-        `);
-        assertThrows(() => t.x, "PythonError", "");
+        const origDebug = pyodide.setDebug(true);
         try {
-            t.x;
-        } catch(e){
-            assert(() => e instanceof pyodide.ffi.PythonError);
+            const t = pyodide.runPython(`
+                from pyodide.ffi import to_js
+                def te(self, *args, **kwargs):
+                    raise Exception(repr(args))
+                class Temp:
+                    __getattr__ = te
+                    __setattr__ = te
+                    __delattr__ = te
+                    __dir__ = te
+                    __call__ = te
+                    __getitem__ = te
+                    __setitem__ = te
+                    __delitem__ = te
+                    __iter__ = te
+                    __len__ = te
+                    __contains__ = te
+                    __await__ = te
+                    __repr__ = te
+                to_js(Temp())
+                Temp()
+            `);
+            assertThrows(() => t.x, "PythonError", "");
+            try {
+                t.x;
+            } catch(e){
+                assert(() => e instanceof pyodide.ffi.PythonError);
+            }
+            assertThrows(() => t.x = 2, "PythonError", "");
+            assertThrows(() => delete t.x, "PythonError", "");
+            assertThrows(() => Object.getOwnPropertyNames(t), "PythonError", "");
+            assertThrows(() => t(), "PythonError", "");
+            assertThrows(() => t.get(1), "PythonError", "");
+            assertThrows(() => t.set(1, 2), "PythonError", "");
+            assertThrows(() => t.delete(1), "PythonError", "");
+            assertThrows(() => t.has(1), "PythonError", "");
+            assertThrows(() => t.length, "PythonError", "");
+            assertThrows(() => t.toString(), "PythonError", "");
+            assertThrows(() => Array.from(t), "PythonError", "");
+            await assertThrowsAsync(async () => await t, "PythonError", "");
+            t.destroy();
+            assertThrows(() => t.type, "Error",
+                "Object has already been destroyed\n" +
+                'The object was of type "Temp" and an error was raised when trying to generate its repr'
+            );
+        } finally {
+            pyodide.setDebug(origDebug);
         }
-        assertThrows(() => t.x = 2, "PythonError", "");
-        assertThrows(() => delete t.x, "PythonError", "");
-        assertThrows(() => Object.getOwnPropertyNames(t), "PythonError", "");
-        assertThrows(() => t(), "PythonError", "");
-        assertThrows(() => t.get(1), "PythonError", "");
-        assertThrows(() => t.set(1, 2), "PythonError", "");
-        assertThrows(() => t.delete(1), "PythonError", "");
-        assertThrows(() => t.has(1), "PythonError", "");
-        assertThrows(() => t.length, "PythonError", "");
-        assertThrows(() => t.toString(), "PythonError", "");
-        assertThrows(() => Array.from(t), "PythonError", "");
-        await assertThrowsAsync(async () => await t, "PythonError", "");
-        t.destroy();
-        assertThrows(() => t.type, "Error",
-            "Object has already been destroyed\n" +
-            'The object was of type "Temp" and an error was raised when trying to generate its repr'
-        );
         """
     )