STINNER Victor added the comment: > Benchmarking results look nice, but despite the fact that this patch is only small part of issue26814, it looks to me larger that it could be.
Oh I failed to express my intent. This initial patch is not expected to introduce any speedup. In fact I noticed major performance regressions on the CPython benchmark suite using my full fastcall patch. It took me time to understand that they are more issues with benchmarks than my work. This minimum patch only adds new functions but don't really use them. I patched a few functions to show how the new functions can be used. I spent most of my time just to ensure that the minimum patch doesn't introduce performance regression. > 1. The patch includes two parts: adding _PyObject_FastCall() and adding PyObject_CallNoArg() and PyObject_CallArg1(). How large the role of latter functions in the speed up? See my remark above, no speedup is expected. Do you suggest to not add these 2 new functions? Since they are well defined and simple, I chose to make them public. Their API is nicer than _PyObject_Call(). > Can existing function PyObject_Call() be optimized to achieve a comparable benefit? Sorry, I don't understand. This function requires a tuple. The whole purpose of my patch is to avoid temporary tuples. In my full patch, PyObject_Call() calls _PyObject_FastCall() in most cases. > 2. I think that supporting keyword arguments in _PyObject_FastCall() doesn't make much sense now. Well, I can add support for keyword arguments later and start with an assertion (fail if they are used). But I really need them in the API, and I don't want to change to API later. I plan to add a new METH_FASTCALL calling convention for C functions. I would prefer to not have two new calling conventions, but use Argument Clinic to emit efficient code to parse arguments. > Calling with keyword arguments adds such much overhead, that it dwarfs the benefit of avoiding the creation of one tuple. I think that the patch can be simpler if drop the support of keyword arguments. Keyword arguments are optional. Having support for them cost nothing when they are not used. > 3. The patch adds two files for one function _PyStack_AsTuple(). I would prefer something like _PyTuple_FromArray(). It could be used in other places, not just in argument parsing. I really want to have a "pystack" API. In this patch, the new file looks useless, but in the full patch there are many functions including a few complex functions. I prefer to add the file now and complete it later. I'm limited by Mercurial and our workflow (tools), it would be much easier to explain my work using a patch serie, but it's not possible to publish a patch serie... ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue27128> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
