llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) <details> <summary>Changes</summary> b05a5d09547b added an arity-trimming step to the shared `ScriptedPythonInterface::Dispatch`: extensions are now allowed to define methods with trailing parameters as optional (`num_children(self)` vs. `num_children(self, max_count)`), so before calling into a method, `Dispatch` needs to know how many positional arguments it actually accepts and drop any trailing ones we'd otherwise pass. That check calls `PythonCallable::GetArgInfo`, which ran a whole embedded Python script through `inspect.signature` on every call, since every scripted-extension dispatch goes through it. Everything `GetArgInfo` needs is available as plain data attributes, with no Python bytecode execution required: `__func__`/`__self__` to unwrap bound methods, `__call__`/`__init__` to resolve classes and callable instances, and `__code__`'s `co_argcount`/`co_flags` for the actual positional-argument count and varargs bit. This removes the `inspect.signature` cost entirely rather than just amortizing it, and closes the C-stack-overflow path outright. rdar://183776556 --- Full diff: https://github.com/llvm/llvm-project/pull/213378.diff 1 Files Affected: - (modified) lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp (+60-32) ``````````diff diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index fba98abf9e83b..7ac8085337e94 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -818,43 +818,71 @@ bool PythonCallable::Check(PyObject *py_obj) { return PyCallable_Check(py_obj); } -static const char get_arg_info_script[] = R"( -from inspect import signature, Parameter, ismethod -from collections import namedtuple -ArgInfo = namedtuple('ArgInfo', ['count', 'has_varargs']) -def main(f): - count = 0 - varargs = False - for parameter in signature(f).parameters.values(): - kind = parameter.kind - if kind in (Parameter.POSITIONAL_ONLY, - Parameter.POSITIONAL_OR_KEYWORD): - count += 1 - elif kind == Parameter.VAR_POSITIONAL: - varargs = True - elif kind in (Parameter.KEYWORD_ONLY, - Parameter.VAR_KEYWORD): - pass - else: - raise Exception(f'unknown parameter kind: {kind}') - return ArgInfo(count, varargs) -)"; - Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const { ArgInfo result = {}; if (!IsValid()) return nullDeref(); - // no need to synchronize access to this global, we already have the GIL - static PythonScript get_arg_info(get_arg_info_script); - Expected<PythonObject> pyarginfo = get_arg_info(*this); - if (!pyarginfo) - return pyarginfo.takeError(); - long long count = - cantFail(As<long long>(pyarginfo.get().GetAttribute("count"))); - bool has_varargs = - cantFail(As<bool>(pyarginfo.get().GetAttribute("has_varargs"))); - result.max_positional_args = has_varargs ? ArgInfo::UNBOUNDED : count; + // Resolve to the underlying plain function. Bound instance/class methods + // expose it via `__func__`, with `__self__` telling us the self/cls + // argument is supplied automatically and shouldn't be counted below. A + // class used as a constructor (e.g. `CreatePluginObject` resolving a + // scripted extension's class object) has neither: calling it invokes + // `__init__`, which always takes an implicit `self`. A plain instance + // used as a callable (defining `__call__` instead of being a function + // itself, e.g. `command script add -f some_module.some_callable_obj`) + // also has neither, but should be introspected via `__call__`, not the + // unrelated `__init__` used to construct that instance in the first + // place. + PythonObject func = *this; + bool implicit_first_arg = false; + if (HasAttribute("__self__")) { + implicit_first_arg = true; + Expected<PythonObject> func_or_err = GetAttribute("__func__"); + if (!func_or_err) + return func_or_err.takeError(); + func = func_or_err.get(); + } else if (!HasAttribute("__code__")) { + implicit_first_arg = true; + Expected<PythonObject> unwrapped_or_err = PyType_Check(m_py_obj) + ? GetAttribute("__init__") + : GetAttribute("__call__"); + if (!unwrapped_or_err) + return unwrapped_or_err.takeError(); + func = unwrapped_or_err.get(); + // `__call__` resolved via an instance is itself a bound method, so + // recurse once to unwrap it the same way as any other bound method. + if (func.HasAttribute("__self__")) { + Expected<PythonObject> inner_func_or_err = func.GetAttribute("__func__"); + if (!inner_func_or_err) + return inner_func_or_err.takeError(); + func = inner_func_or_err.get(); + } + } + + Expected<PythonObject> code_or_err = func.GetAttribute("__code__"); + if (!code_or_err) + return code_or_err.takeError(); + PythonObject code = code_or_err.get(); + + Expected<long long> argcount = + As<long long>(code.GetAttribute("co_argcount")); + if (!argcount) + return argcount.takeError(); + Expected<long long> flags = As<long long>(code.GetAttribute("co_flags")); + if (!flags) + return flags.takeError(); + + // CO_VARARGS: set when the function accepts a `*args`-style catch-all. + // This flag value is part of the long-standing `inspect`/`dis` module + // constants and hasn't changed since Python 2. + constexpr long long CO_VARARGS = 0x04; + if (*flags & CO_VARARGS) { + result.max_positional_args = ArgInfo::UNBOUNDED; + } else { + long long count = *argcount - (implicit_first_arg ? 1 : 0); + result.max_positional_args = count > 0 ? static_cast<unsigned>(count) : 0; + } return result; } `````````` </details> https://github.com/llvm/llvm-project/pull/213378 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
