From 4cd50fa68d54d5364045d26cd1e49e1254a21fb7 Mon Sep 17 00:00:00 2001 From: David Rose Date: Wed, 7 Jan 2009 20:05:57 +0000 Subject: [PATCH] fix performance issues with implicit parameter coercion --- .../interfaceMakerPythonNative.cxx | 28 +++-- dtool/src/interrogatedb/py_panda.cxx | 115 +++++++++++------- dtool/src/interrogatedb/py_panda.h | 2 + 3 files changed, 93 insertions(+), 52 deletions(-) diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.cxx b/dtool/src/interrogate/interfaceMakerPythonNative.cxx index 0e2346337f..b5bd404523 100755 --- a/dtool/src/interrogate/interfaceMakerPythonNative.cxx +++ b/dtool/src/interrogate/interfaceMakerPythonNative.cxx @@ -2134,6 +2134,8 @@ write_function_forset(ostream &out, InterfaceMaker::Object *obj, << "PyObject *coerced = NULL;\n"; indent(out,indent_level) << "PyObject **coerced_ptr = NULL;\n"; + indent(out,indent_level) + << "bool report_errors = false;\n"; indent(out,indent_level) << "while (true) {\n"; indent_level += 2; @@ -2219,16 +2221,28 @@ write_function_forset(ostream &out, InterfaceMaker::Object *obj, if (coercion_possible) { // Try again, this time with coercion enabled. indent(out,indent_level) - << "if (coerced_ptr != NULL) {\n"; + << "if (coerced_ptr == NULL && !report_errors) {\n"; indent(out,indent_level + 2) - << "break;\n"; + << "coerced_ptr = &coerced;\n"; + indent(out,indent_level + 2) + << "continue;\n"; indent(out,indent_level) << "}\n"; - + + // No dice. Go back one more time, and this time get the error + // message. indent(out,indent_level) - << "PyErr_Clear();\n"; + << "if (!report_errors) {\n"; + indent(out,indent_level + 2) + << "report_errors = true;\n"; + indent(out,indent_level + 2) + << "continue;\n"; indent(out,indent_level) - << "coerced_ptr = &coerced;\n"; + << "}\n"; + + // We've been through three times. We're done. + indent(out,indent_level) + << "break;\n"; indent_level -= 2; indent(out,indent_level) @@ -2453,9 +2467,9 @@ write_function_instance(ostream &out, InterfaceMaker::Object *obj, if (coercion_possible && !is_copy_constructor) { // We never attempt to coerce a copy constructor parameter. // That would lead to infinite recursion. - str << ", coerced_ptr"; + str << ", coerced_ptr, report_errors"; } else { - str << ", NULL"; + str << ", NULL, true"; } str << ");\n"; diff --git a/dtool/src/interrogatedb/py_panda.cxx b/dtool/src/interrogatedb/py_panda.cxx index 0d491ab895..3fc14bedba 100644 --- a/dtool/src/interrogatedb/py_panda.cxx +++ b/dtool/src/interrogatedb/py_panda.cxx @@ -94,10 +94,24 @@ attempt_coercion(PyObject *self, Dtool_PyTypedObject *classdef, // temporary object. Weird. Py_DECREF(obj); } + + // Clear the error returned by the coercion constructor. It's not + // the error message we want to report. + PyErr_Clear(); } return NULL; } +// Temporary function to preserve backward compatibility. +void * +DTOOL_Call_GetPointerThisClass(PyObject *self, Dtool_PyTypedObject *classdef, + int param, const string &function_name, bool const_ok, + PyObject **coerced) { + return DTOOL_Call_GetPointerThisClass(self, classdef, + param, function_name, const_ok, + coerced, true); +} + //////////////////////////////////////////////////////////////////// // Function: DTOOL_Call_GetPointerThisClass // Description: Extracts the C++ pointer for an object, given its @@ -140,7 +154,10 @@ attempt_coercion(PyObject *self, Dtool_PyTypedObject *classdef, void * DTOOL_Call_GetPointerThisClass(PyObject *self, Dtool_PyTypedObject *classdef, int param, const string &function_name, bool const_ok, - PyObject **coerced) { + PyObject **coerced, bool report_errors) { + if (PyErr_Occurred()) { + return NULL; + } if (self != NULL) { if (DtoolCanThisBeAPandaInstance(self)) { Dtool_PyTypedObject *my_type = ((Dtool_PyInstDef *)self)->_My_Type; @@ -150,34 +167,38 @@ DTOOL_Call_GetPointerThisClass(PyObject *self, Dtool_PyTypedObject *classdef, return result; } - ostringstream str; - str << function_name << "() argument " << param << " may not be const"; - string msg = str.str(); - PyErr_SetString(PyExc_TypeError, msg.c_str()); + if (report_errors) { + ostringstream str; + str << function_name << "() argument " << param << " may not be const"; + string msg = str.str(); + PyErr_SetString(PyExc_TypeError, msg.c_str()); + } } else { - ostringstream str; - str << function_name << "() argument " << param << " must be "; - - - PyObject *fname = PyObject_GetAttrString((PyObject *)classdef, "__name__"); - if (fname != (PyObject *)NULL) { - str << PyString_AsString(fname); - Py_DECREF(fname); - } else { - str << classdef->_name; + if (report_errors) { + ostringstream str; + str << function_name << "() argument " << param << " must be "; + + + PyObject *fname = PyObject_GetAttrString((PyObject *)classdef, "__name__"); + if (fname != (PyObject *)NULL) { + str << PyString_AsString(fname); + Py_DECREF(fname); + } else { + str << classdef->_name; + } + + PyObject *tname = PyObject_GetAttrString((PyObject *)self->ob_type, "__name__"); + if (tname != (PyObject *)NULL) { + str << ", not " << PyString_AsString(tname); + Py_DECREF(tname); + } else { + str << ", not " << my_type->_name; + } + + string msg = str.str(); + PyErr_SetString(PyExc_TypeError, msg.c_str()); } - - PyObject *tname = PyObject_GetAttrString((PyObject *)self->ob_type, "__name__"); - if (tname != (PyObject *)NULL) { - str << ", not " << PyString_AsString(tname); - Py_DECREF(tname); - } else { - str << ", not " << my_type->_name; - } - - string msg = str.str(); - PyErr_SetString(PyExc_TypeError, msg.c_str()); } } else { @@ -189,28 +210,32 @@ DTOOL_Call_GetPointerThisClass(PyObject *self, Dtool_PyTypedObject *classdef, } // Coercion failed. - ostringstream str; - str << function_name << "() argument " << param << " must be "; - - PyObject *fname = PyObject_GetAttrString((PyObject *)classdef, "__name__"); - if (fname != (PyObject *)NULL) { - str << PyString_AsString(fname); - Py_DECREF(fname); - } else { - str << classdef->_name; + if (report_errors) { + ostringstream str; + str << function_name << "() argument " << param << " must be "; + + PyObject *fname = PyObject_GetAttrString((PyObject *)classdef, "__name__"); + if (fname != (PyObject *)NULL) { + str << PyString_AsString(fname); + Py_DECREF(fname); + } else { + str << classdef->_name; + } + + PyObject *tname = PyObject_GetAttrString((PyObject *)self->ob_type, "__name__"); + if (tname != (PyObject *)NULL) { + str << ", not " << PyString_AsString(tname); + Py_DECREF(tname); + } + + string msg = str.str(); + PyErr_SetString(PyExc_TypeError, msg.c_str()); } - - PyObject *tname = PyObject_GetAttrString((PyObject *)self->ob_type, "__name__"); - if (tname != (PyObject *)NULL) { - str << ", not " << PyString_AsString(tname); - Py_DECREF(tname); - } - - string msg = str.str(); - PyErr_SetString(PyExc_TypeError, msg.c_str()); } } else { - PyErr_SetString(PyExc_TypeError, "Self Is Null"); + if (report_errors) { + PyErr_SetString(PyExc_TypeError, "Self Is Null"); + } } return NULL; diff --git a/dtool/src/interrogatedb/py_panda.h b/dtool/src/interrogatedb/py_panda.h index 21cb39c5e2..45fcd37283 100755 --- a/dtool/src/interrogatedb/py_panda.h +++ b/dtool/src/interrogatedb/py_panda.h @@ -364,6 +364,8 @@ EXPCL_DTOOLCONFIG bool DtoolCanThisBeAPandaInstance(PyObject *self); EXPCL_DTOOLCONFIG void DTOOL_Call_ExtractThisPointerForType(PyObject *self, Dtool_PyTypedObject * classdef, void ** answer); +EXPCL_DTOOLCONFIG void * DTOOL_Call_GetPointerThisClass(PyObject *self, Dtool_PyTypedObject *classdef, int param, const string &function_name, bool const_ok, PyObject **coerced, bool report_errors); + EXPCL_DTOOLCONFIG void * DTOOL_Call_GetPointerThisClass(PyObject *self, Dtool_PyTypedObject *classdef, int param, const string &function_name, bool const_ok, PyObject **coerced); EXPCL_DTOOLCONFIG void * DTOOL_Call_GetPointerThis(PyObject *self);